[GitHub] [arrow] LouisClt commented on a diff in pull request #13737: Add scalar casts to string types for list based types

[GitBox <gi...@apache.org>]

LouisClt commented on code in PR #13737:
URL: https://github.com/apache/arrow/pull/13737#discussion_r932964877


##########
cpp/src/arrow/scalar.cc:
##########
@@ -1041,6 +1041,24 @@ Status CastImpl(const StructScalar& from, StringScalar* to) {
   return Status::OK();
 }
 
+// list based types (list, large list and map (fixed sized list too)) to string
+Status CastImpl(const BaseListScalar& from, StringScalar* to) {
+  std::stringstream ss;
+  ss << from.type->ToString() << "{";

Review Comment:
   Hello, yes maybe indeed it will be better, and more coherent with languages such as Python (I see there was also a JSON format representing Arrays, maybe it will be more coherent with that too).
   
   As for the tests, maybe something like that will do :
   ```
   ASSERT_OK_AND_ASSIGN(auto str,
         ListScalar(ArrayFromJSON(int16(), "[1, 2, 5]"), list(int16())).CastTo(utf8()));
       EXPECT_EQ(*str, StringScalar("list<item: int16>[1, 2, 3]"));
   ```
       
   But I must admit I can't test this for the moment, I first need to set up better my dev environment for Arrow, because for the moment I do not build the tests (I am on windows building with vcpkg)
   So feel free to help as you want.
   
   I accepted your suggestion, which makes the code simpler indeed (and might correct the workflows that did not pass with the first version)



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org