You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/06/15 18:35:28 UTC

[GitHub] [arrow] fsaintjacques commented on a change in pull request #7435: ARROW-8779: [R] Implement conversion to List

fsaintjacques commented on a change in pull request #7435:
URL: https://github.com/apache/arrow/pull/7435#discussion_r440369138



##########
File path: r/src/array_from_vector.cpp
##########
@@ -201,6 +201,39 @@ struct VectorToArrayConverter {
     return Status::OK();
   }
 
+  template <typename T>
+  arrow::enable_if_t<arrow::is_struct_type<T>::value, Status> Visit(const T& type) {
+    using BuilderType = typename TypeTraits<T>::BuilderType;
+    ARROW_RETURN_IF(!Rf_inherits(x, "data.frame"),
+                    Status::RError("Expecting a data frame"));
+
+    auto* struct_builder = checked_cast<BuilderType*>(builder);
+
+    int64_t n = 0;
+    if (XLENGTH(x) > 0) {
+      // TODO: need a more generic way to get the vec_size(<data.frame>)
+      //       perhaps using vctrs::::short_vec_size
+      n = XLENGTH(VECTOR_ELT(x, 0));
+    }
+    RETURN_NOT_OK(struct_builder->Reserve(n));
+
+    // TODO: double check but seems fine that no value is NULL

Review comment:
       You can remove the TODO, this is correct.

##########
File path: r/tests/testthat/test-Array.R
##########
@@ -445,6 +445,9 @@ test_that("Array$create() handles vector -> list arrays (ARROW-7662)", {
   expect_array_roundtrip(list(character(0)), list_of(utf8()))
   expect_array_roundtrip(list("itsy", c("bitsy", "spider"), c("is")), list_of(utf8()))
   expect_array_roundtrip(list("itsy", character(0), c("bitsy", "spider", NA_character_), c("is")), list_of(utf8()))
+
+  # struct
+  expect_array_roundtrip(list(tibble::tibble(a = integer(0))), list_of(struct(a = int32())))

Review comment:
       Add a bit more columns: float, string, bool. Also add a failing case where columns are not of the same size.




----------------------------------------------------------------
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.

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