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 2021/12/14 18:22:47 UTC

[GitHub] [arrow] wjones127 opened a new pull request #11956: ARROW-10456: [R] Implement MapType

wjones127 opened a new pull request #11956:
URL: https://github.com/apache/arrow/pull/11956


   This PR adds `MapType` and `MapArray` to R implementation. 


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



[GitHub] [arrow] ursabot commented on pull request #11956: ARROW-10456: [R] Implement MapType and MapArray

Posted by GitBox <gi...@apache.org>.
ursabot commented on pull request #11956:
URL: https://github.com/apache/arrow/pull/11956#issuecomment-1024422478


   Benchmark runs are scheduled for baseline = c5b757fe607b1e5824053da279a727e35e877e0a and contender = f92219d05e0255157f628baa445824a96ff94ada. f92219d05e0255157f628baa445824a96ff94ada is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Scheduled] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/f132a0b9475c44f5af7b55a00fd20ad1...48a1b684f6f74b1f8270b5ec34aa74d1/)
   [Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/087d322e784e480b8f8561bb34ebd35e...4201d0e3e06c419aa776e09fb739819e/)
   [Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/42ec3d3a5b8b4f39b064f1be8303db71...8c95fb8fe44f4e67824b020110658a70/)
   Supported benchmarks:
   ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
   ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
   ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
   


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



[GitHub] [arrow] ursabot edited a comment on pull request #11956: ARROW-10456: [R] Implement MapType and MapArray

Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #11956:
URL: https://github.com/apache/arrow/pull/11956#issuecomment-1024422478


   Benchmark runs are scheduled for baseline = c5b757fe607b1e5824053da279a727e35e877e0a and contender = f92219d05e0255157f628baa445824a96ff94ada. f92219d05e0255157f628baa445824a96ff94ada is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/f132a0b9475c44f5af7b55a00fd20ad1...48a1b684f6f74b1f8270b5ec34aa74d1/)
   [Failed :arrow_down:0.0% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/087d322e784e480b8f8561bb34ebd35e...4201d0e3e06c419aa776e09fb739819e/)
   [Finished :arrow_down:0.3% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/42ec3d3a5b8b4f39b064f1be8303db71...8c95fb8fe44f4e67824b020110658a70/)
   Supported benchmarks:
   ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
   ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
   ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
   


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



[GitHub] [arrow] wjones127 edited a comment on pull request #11956: ARROW-10456: [R] Implement MapType and MapArray

Posted by GitBox <gi...@apache.org>.
wjones127 edited a comment on pull request #11956:
URL: https://github.com/apache/arrow/pull/11956#issuecomment-1024317901


   > What do the map types look like when they get pulled into R? I don't think I saw a test that showed that (but maybe I missed it)? Would if make sense to have a test like that?
   
   ~You're right, I need to add a test for that~. Right now, they should just use the same code paths as ListArray.


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



[GitHub] [arrow] wjones127 commented on a change in pull request #11956: ARROW-10456: [R] Implement MapType

Posted by GitBox <gi...@apache.org>.
wjones127 commented on a change in pull request #11956:
URL: https://github.com/apache/arrow/pull/11956#discussion_r773375168



##########
File path: r/tests/testthat/test-parquet.R
##########
@@ -322,3 +340,13 @@ test_that("ParquetFileWrite chunk_size calculation doesn't have integer overflow
   # but our max_chunks is respected
   expect_equal(calculate_chunk_size(101, 1, 25, 2), 51)
 })
+
+# Wait on ARROW-13735 for this

Review comment:
       I'd like us to be able to validate we can read all the test parquet files after this. (I think parquet support is the main motivation for MapType support; not sure it's much use otherwise.) Waiting on #11855, which solves a bug affecting one of these.




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



[GitHub] [arrow] jonkeane commented on a change in pull request #11956: ARROW-10456: [R] Implement MapType and MapArray

Posted by GitBox <gi...@apache.org>.
jonkeane commented on a change in pull request #11956:
URL: https://github.com/apache/arrow/pull/11956#discussion_r794658143



##########
File path: r/tests/testthat/test-Array.R
##########
@@ -572,6 +572,29 @@ test_that("Array$create() handles vector -> list arrays (ARROW-7662)", {
   expect_error(Array$create(list(df)))
 })
 
+test_that("Array$create() handles list of dataframes -> map arrays", {
+  # Should be able to create an empty map with a type hint.
+  expect_r6_class(Array$create(list(), type = map_of(utf8(), boolean())), "MapArray")
+
+  # MapType is alias for List<Struct<keys, values>>
+  data <- list(data.frame(key = c("a", "b"), value = c(1, 2), stringsAsFactors = FALSE),
+               data.frame(key = c("a", "c"), value = c(4, 7), stringsAsFactors = FALSE))
+  arr <- Array$create(data, type = map_of(utf8(), int32()))
+
+  expect_r6_class(arr, "MapArray")
+  expect_as_vector(arr, data, ignore_attr = TRUE)

Review comment:
       Aaah yes, of course! Thanks




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



[GitHub] [arrow] wjones127 commented on a change in pull request #11956: ARROW-10456: [R] Implement MapType

Posted by GitBox <gi...@apache.org>.
wjones127 commented on a change in pull request #11956:
URL: https://github.com/apache/arrow/pull/11956#discussion_r773375793



##########
File path: r/R/array.R
##########
@@ -328,3 +328,17 @@ is.Array <- function(x, type = NULL) { # nolint
   }
   is_it
 }
+
+#' @rdname array
+#' @usage NULL
+#' @format NULL
+#' @export
+MapArray <- R6Class("MapArray",
+  inherit = ListArray,
+  public = list(
+    keys = function() MapArray__keys(self),
+    items = function() MapArray__items(self),
+    keys_nested = function() MapArray__keys_nested(self),
+    items_nested = function() MapArray__items_nested(self)

Review comment:
       These methods don't exist in other implementations, but I thought they would be useful to R users.




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



[GitHub] [arrow] wjones127 commented on a change in pull request #11956: ARROW-10456: [R] Implement MapType

Posted by GitBox <gi...@apache.org>.
wjones127 commented on a change in pull request #11956:
URL: https://github.com/apache/arrow/pull/11956#discussion_r773375793



##########
File path: r/R/array.R
##########
@@ -328,3 +328,17 @@ is.Array <- function(x, type = NULL) { # nolint
   }
   is_it
 }
+
+#' @rdname array
+#' @usage NULL
+#' @format NULL
+#' @export
+MapArray <- R6Class("MapArray",
+  inherit = ListArray,
+  public = list(
+    keys = function() MapArray__keys(self),
+    items = function() MapArray__items(self),
+    keys_nested = function() MapArray__keys_nested(self),
+    items_nested = function() MapArray__items_nested(self)

Review comment:
       These methods don't exist in other implementations, but I thought they would more intuitive to R users than manipulating offsets vectors to get the nested versions. LMK if there is a better name for these.




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



[GitHub] [arrow] ursabot edited a comment on pull request #11956: ARROW-10456: [R] Implement MapType and MapArray

Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #11956:
URL: https://github.com/apache/arrow/pull/11956#issuecomment-1024422478


   Benchmark runs are scheduled for baseline = c5b757fe607b1e5824053da279a727e35e877e0a and contender = f92219d05e0255157f628baa445824a96ff94ada. f92219d05e0255157f628baa445824a96ff94ada is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/f132a0b9475c44f5af7b55a00fd20ad1...48a1b684f6f74b1f8270b5ec34aa74d1/)
   [Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/087d322e784e480b8f8561bb34ebd35e...4201d0e3e06c419aa776e09fb739819e/)
   [Finished :arrow_down:0.3% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/42ec3d3a5b8b4f39b064f1be8303db71...8c95fb8fe44f4e67824b020110658a70/)
   Supported benchmarks:
   ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
   ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
   ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
   


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



[GitHub] [arrow] github-actions[bot] commented on pull request #11956: ARROW-10456: [R] Implement MapType

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11956:
URL: https://github.com/apache/arrow/pull/11956#issuecomment-993857326


   https://issues.apache.org/jira/browse/ARROW-10456


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



[GitHub] [arrow] wjones127 commented on pull request #11956: ARROW-10456: [R] Implement MapType and MapArray

Posted by GitBox <gi...@apache.org>.
wjones127 commented on pull request #11956:
URL: https://github.com/apache/arrow/pull/11956#issuecomment-1024317901


   > What do the map types look like when they get pulled into R? I don't think I saw a test that showed that (but maybe I missed it)? Would if make sense to have a test like that?
   
   You're right, I need to add a test for that. Right now, they should just use the same code paths as ListArray.


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



[GitHub] [arrow] jonkeane closed pull request #11956: ARROW-10456: [R] Implement MapType and MapArray

Posted by GitBox <gi...@apache.org>.
jonkeane closed pull request #11956:
URL: https://github.com/apache/arrow/pull/11956


   


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



[GitHub] [arrow] wjones127 commented on a change in pull request #11956: ARROW-10456: [R] Implement MapType and MapArray

Posted by GitBox <gi...@apache.org>.
wjones127 commented on a change in pull request #11956:
URL: https://github.com/apache/arrow/pull/11956#discussion_r779876232



##########
File path: r/src/symbols.cpp
##########
@@ -75,6 +75,10 @@ SEXP data::classes_arrow_large_list = precious(cpp11::writable::strings(
 SEXP data::classes_arrow_fixed_size_list = precious(cpp11::writable::strings(
     {"arrow_fixed_size_list", "vctrs_list_of", "vctrs_vctr", "list"}));
 
+// TODO: do we even need this?
+SEXP data::classes_arrow_map = precious(
+    cpp11::writable::strings({"arrow_map", "arrow_list", "vctrs_list_of", "vctrs_vctr", "list"}));
+

Review comment:
       Okay. I think I want to leave that for a follow-up. IMO most of the value in implementing map arrays is reading files and streams from other languages; I don't think it's as meaningful a type in R.




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



[GitHub] [arrow] wjones127 commented on a change in pull request #11956: ARROW-10456: [R] Implement MapType and MapArray

Posted by GitBox <gi...@apache.org>.
wjones127 commented on a change in pull request #11956:
URL: https://github.com/apache/arrow/pull/11956#discussion_r794653813



##########
File path: r/tests/testthat/test-Array.R
##########
@@ -572,6 +572,29 @@ test_that("Array$create() handles vector -> list arrays (ARROW-7662)", {
   expect_error(Array$create(list(df)))
 })
 
+test_that("Array$create() handles list of dataframes -> map arrays", {
+  # Should be able to create an empty map with a type hint.
+  expect_r6_class(Array$create(list(), type = map_of(utf8(), boolean())), "MapArray")
+
+  # MapType is alias for List<Struct<keys, values>>
+  data <- list(data.frame(key = c("a", "b"), value = c(1, 2), stringsAsFactors = FALSE),
+               data.frame(key = c("a", "c"), value = c(4, 7), stringsAsFactors = FALSE))
+  arr <- Array$create(data, type = map_of(utf8(), int32()))
+
+  expect_r6_class(arr, "MapArray")
+  expect_as_vector(arr, data, ignore_attr = TRUE)

Review comment:
       @jonkeane Actually there is a test here, which tests the round trip. Maps are lists of dataframes with `key` and `value` columns. 




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



[GitHub] [arrow] ursabot edited a comment on pull request #11956: ARROW-10456: [R] Implement MapType and MapArray

Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #11956:
URL: https://github.com/apache/arrow/pull/11956#issuecomment-1024422478


   Benchmark runs are scheduled for baseline = c5b757fe607b1e5824053da279a727e35e877e0a and contender = f92219d05e0255157f628baa445824a96ff94ada. f92219d05e0255157f628baa445824a96ff94ada is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/f132a0b9475c44f5af7b55a00fd20ad1...48a1b684f6f74b1f8270b5ec34aa74d1/)
   [Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/087d322e784e480b8f8561bb34ebd35e...4201d0e3e06c419aa776e09fb739819e/)
   [Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/42ec3d3a5b8b4f39b064f1be8303db71...8c95fb8fe44f4e67824b020110658a70/)
   Supported benchmarks:
   ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python. Runs only benchmarks with cloud = True
   ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
   ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
   


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



[GitHub] [arrow] wjones127 commented on pull request #11956: ARROW-10456: [R] Implement MapType and MapArray

Posted by GitBox <gi...@apache.org>.
wjones127 commented on pull request #11956:
URL: https://github.com/apache/arrow/pull/11956#issuecomment-1006923343


   Seems to be failing on Windows Rtools 3.5. Looking into it locally...


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



[GitHub] [arrow] wjones127 commented on a change in pull request #11956: ARROW-10456: [R] Implement MapType and MapArray

Posted by GitBox <gi...@apache.org>.
wjones127 commented on a change in pull request #11956:
URL: https://github.com/apache/arrow/pull/11956#discussion_r779201095



##########
File path: r/src/symbols.cpp
##########
@@ -75,6 +75,10 @@ SEXP data::classes_arrow_large_list = precious(cpp11::writable::strings(
 SEXP data::classes_arrow_fixed_size_list = precious(cpp11::writable::strings(
     {"arrow_fixed_size_list", "vctrs_list_of", "vctrs_vctr", "list"}));
 
+// TODO: do we even need this?
+SEXP data::classes_arrow_map = precious(
+    cpp11::writable::strings({"arrow_map", "arrow_list", "vctrs_list_of", "vctrs_vctr", "list"}));
+

Review comment:
       I deleted this part, as the structs don't seem to have this, and maps are made of structs.




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



[GitHub] [arrow] jonkeane commented on a change in pull request #11956: ARROW-10456: [R] Implement MapType and MapArray

Posted by GitBox <gi...@apache.org>.
jonkeane commented on a change in pull request #11956:
URL: https://github.com/apache/arrow/pull/11956#discussion_r794551527



##########
File path: r/tests/testthat/test-data-type.R
##########
@@ -367,6 +367,46 @@ test_that("list type works as expected", {
   expect_equal(x$value_field, field("item", int32()))
 })
 
+test_that("map type works as expected", {
+  x <- map_of(int32(), utf8())
+  expect_equal(x$id, Type$MAP)
+  expect_equal(x$name, "map")
+  expect_equal(x$ToString(), "map<int32, string>")
+  expect_true(x == x)
+  expect_false(x == null())
+  expect_equal(
+    x$key_field,
+    field("key", int32(), nullable = FALSE)
+  )
+  expect_equal(
+    x$item_field,
+    field("value", utf8())
+  )
+  expect_equal(x$key_type, int32())
+  expect_equal(x$item_type, string())
+  # TODO: (ARROW-15102): Enable constructing StructTypes with non-nullable fields, so
+  # we can make this comparison:
+  # nolint start (lintr doesn't like commented out code)
+  # expect_equal(x$value_type, struct(key = x$key_field, value = x$item_field))
+  # nolint end

Review comment:
       ```suggestion
     # expect_equal(x$value_type, struct(key = x$key_field, value = x$item_field)) # nolint
   ```
   
   You don't have to do it this way (the start/end also works) but this _should_ work unless lintr's parsing doesn't do the right thing




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



[GitHub] [arrow] pitrou commented on pull request #11956: ARROW-10456: [R] Implement MapType and MapArray

Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #11956:
URL: https://github.com/apache/arrow/pull/11956#issuecomment-1023335788


   Some conflicts need to be fixed. Also @thisisnic or @jonkeane can one of you give this a final review?


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



[GitHub] [arrow] wjones127 commented on a change in pull request #11956: ARROW-10456: [R] Implement MapType

Posted by GitBox <gi...@apache.org>.
wjones127 commented on a change in pull request #11956:
URL: https://github.com/apache/arrow/pull/11956#discussion_r773374201



##########
File path: r/src/symbols.cpp
##########
@@ -75,6 +75,10 @@ SEXP data::classes_arrow_large_list = precious(cpp11::writable::strings(
 SEXP data::classes_arrow_fixed_size_list = precious(cpp11::writable::strings(
     {"arrow_fixed_size_list", "vctrs_list_of", "vctrs_vctr", "list"}));
 
+// TODO: do we even need this?
+SEXP data::classes_arrow_map = precious(
+    cpp11::writable::strings({"arrow_map", "arrow_list", "vctrs_list_of", "vctrs_vctr", "list"}));
+

Review comment:
       I'm a little confused on what this does, and if I need it. Arrow's maps are just lists of structs with some extra validation. What behavior does this affect? How is it tested?




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



[GitHub] [arrow] romainfrancois commented on a change in pull request #11956: ARROW-10456: [R] Implement MapType and MapArray

Posted by GitBox <gi...@apache.org>.
romainfrancois commented on a change in pull request #11956:
URL: https://github.com/apache/arrow/pull/11956#discussion_r779376573



##########
File path: r/src/symbols.cpp
##########
@@ -75,6 +75,10 @@ SEXP data::classes_arrow_large_list = precious(cpp11::writable::strings(
 SEXP data::classes_arrow_fixed_size_list = precious(cpp11::writable::strings(
     {"arrow_fixed_size_list", "vctrs_list_of", "vctrs_vctr", "list"}));
 
+// TODO: do we even need this?
+SEXP data::classes_arrow_map = precious(
+    cpp11::writable::strings({"arrow_map", "arrow_list", "vctrs_list_of", "vctrs_vctr", "list"}));
+

Review comment:
       I think this would be useful for *automatic* creating with `Array$create()` without giving `type=`, the same way as e.g. binary types. 




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



[GitHub] [arrow] wjones127 commented on a change in pull request #11956: ARROW-10456: [R] Implement MapType

Posted by GitBox <gi...@apache.org>.
wjones127 commented on a change in pull request #11956:
URL: https://github.com/apache/arrow/pull/11956#discussion_r778417103



##########
File path: r/src/symbols.cpp
##########
@@ -75,6 +75,10 @@ SEXP data::classes_arrow_large_list = precious(cpp11::writable::strings(
 SEXP data::classes_arrow_fixed_size_list = precious(cpp11::writable::strings(
     {"arrow_fixed_size_list", "vctrs_list_of", "vctrs_vctr", "list"}));
 
+// TODO: do we even need this?
+SEXP data::classes_arrow_map = precious(
+    cpp11::writable::strings({"arrow_map", "arrow_list", "vctrs_list_of", "vctrs_vctr", "list"}));
+

Review comment:
       @romainfrancois I'm nearly done with this PR, but stumped on this detail. Do you have advice on this?




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