rok commented on code in PR #36496:
URL: https://github.com/apache/arrow/pull/36496#discussion_r1304319761
##########
cpp/src/arrow/extension/fixed_shape_tensor.cc:
##########
@@ -104,6 +104,38 @@ bool FixedShapeTensorType::ExtensionEquals(const
ExtensionType& other) const {
permutation_equivalent;
}
+std::string FixedShapeTensorType::ToString() const {
+ std::stringstream ss;
+ ss << "extension<" << this->extension_name()
+ << "[value_type=" << value_type_->ToString() << ", shape=[";
+ std::string separator;
+ for (auto v : shape_) {
+ ss << separator << v;
Review Comment:
Using an uninitialized variable here [seems to be
ok](https://stackoverflow.com/questions/17738439/value-and-size-of-an-uninitialized-stdstring-variable-in-c),
but is otherwise probably best avoided.
.
##########
cpp/src/arrow/extension/fixed_shape_tensor.cc:
##########
@@ -104,6 +104,38 @@ bool FixedShapeTensorType::ExtensionEquals(const
ExtensionType& other) const {
permutation_equivalent;
}
+std::string FixedShapeTensorType::ToString() const {
+ std::stringstream ss;
+ ss << "extension<" << this->extension_name()
+ << "[value_type=" << value_type_->ToString() << ", shape=[";
+ std::string separator;
+ for (auto v : shape_) {
+ ss << separator << v;
+ separator = ",";
+ }
+ ss << "]";
+ if (!permutation_.empty()) {
+ ss << ", permutation=[";
+ std::string p_separator;
+ for (auto v : permutation_) {
+ ss << p_separator << v;
+ p_separator = ",";
+ }
+ ss << "]";
+ }
+ if (!dim_names_.empty()) {
+ ss << ", dim_names=[";
+ std::string d_separator;
+ for (std::string v : dim_names_) {
+ ss << d_separator << v;
+ d_separator = ",";
+ }
+ ss << "]";
+ }
+ ss << "]>";
+ return ss.str();
+}
+
Review Comment:
How about we introduce a helper function for all the cases where we're
joining vectors into comma delimited strings?
```diff
diff --git a/cpp/src/arrow/extension/fixed_shape_tensor.cc
b/cpp/src/arrow/extension/fixed_shape_tensor.cc
index a997e8965..40dca52c1 100644
--- a/cpp/src/arrow/extension/fixed_shape_tensor.cc
+++ b/cpp/src/arrow/extension/fixed_shape_tensor.cc
@@ -27,6 +27,7 @@
#include "arrow/util/int_util_overflow.h"
#include "arrow/util/logging.h"
#include "arrow/util/sort.h"
+#include "arrow/util/string.h"
#include <rapidjson/document.h>
#include <rapidjson/writer.h>
@@ -107,30 +108,13 @@ bool FixedShapeTensorType::ExtensionEquals(const
ExtensionType& other) const {
std::string FixedShapeTensorType::ToString() const {
std::stringstream ss;
ss << "extension<" << this->extension_name()
- << "[value_type=" << value_type_->ToString() << ", shape=[";
- std::string separator;
- for (auto v : shape_) {
- ss << separator << v;
- separator = ",";
- }
- ss << "]";
+ << "[value_type=" << value_type_->ToString();
+ ss << ", shape=[" << internal::JoinStrings(shape_, ",") << "]";
if (!permutation_.empty()) {
- ss << ", permutation=[";
- std::string p_separator;
- for (auto v : permutation_) {
- ss << p_separator << v;
- p_separator = ",";
- }
- ss << "]";
+ ss << ", permutation=[" << internal::JoinStrings(permutation_, ",") <<
"]";
}
if (!dim_names_.empty()) {
- ss << ", dim_names=[";
- std::string d_separator;
- for (std::string v : dim_names_) {
- ss << d_separator << v;
- d_separator = ",";
- }
- ss << "]";
+ ss << ", dim_names=[" << internal::JoinStrings(dim_names_, ",") << "]";
}
ss << "]>";
return ss.str();
diff --git a/cpp/src/arrow/util/string.cc b/cpp/src/arrow/util/string.cc
index 2055b4f47..f33774091 100644
--- a/cpp/src/arrow/util/string.cc
+++ b/cpp/src/arrow/util/string.cc
@@ -135,6 +135,13 @@ std::string JoinStrings(const std::vector<std::string>&
strings,
return JoinStringLikes(strings, delimiter);
}
+std::string JoinStrings(const std::vector<int64_t>& values,
std::string_view delimiter) {
+ std::vector<std::string> strings;
+ std::transform(values.begin(), values.end(), std::back_inserter(strings),
+ [](const int64_t v) { return std::to_string(v); });
+ return JoinStringLikes(strings, delimiter);
+}
+
static constexpr bool IsWhitespace(char c) { return c == ' ' || c == '\t'; }
std::string TrimString(std::string value) {
diff --git a/cpp/src/arrow/util/string.h b/cpp/src/arrow/util/string.h
index d9777efc5..d666ccd42 100644
--- a/cpp/src/arrow/util/string.h
+++ b/cpp/src/arrow/util/string.h
@@ -77,6 +77,10 @@ ARROW_EXPORT
std::string JoinStrings(const std::vector<std::string>& strings,
std::string_view delimiter);
+/// \brief Join integer values with a delimiter as strings
+ARROW_EXPORT
+std::string JoinStrings(const std::vector<int64_t>& values,
std::string_view delimiter);
+
/// \brief Trim whitespace from left and right sides of string
ARROW_EXPORT
std::string TrimString(std::string value);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
