[GitHub] [arrow] paleolimbot commented on a diff in pull request #13851: ARROW-14999: [C++] Don't check field name in ListType Equals()

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

paleolimbot commented on code in PR #13851:
URL: https://github.com/apache/arrow/pull/13851#discussion_r1034991796


##########
java/c/src/test/python/integration_tests.py:
##########
@@ -16,6 +16,7 @@
 # under the License.
 
 import decimal
+from email.policy import strict

Review Comment:
   This seems out of place?



##########
r/tests/testthat/test-data-type.R:
##########
@@ -365,6 +365,20 @@ test_that("list type works as expected", {
   )
   expect_equal(x$value_type, int32())
   expect_equal(x$value_field, field("item", int32()))
+
+  # nullability matters in comparison
+  expect_false(x$Equals(list_of(field("item", int32(), nullable = FALSE))))
+
+  # field names don't matter by default
+  other_name <- list_of(field("other", int32()))
+  expect_equal(x, other_name)
+  expect_false(x$Equals(other_name, check_internal_field_names = TRUE))
+
+  # TODO(ARROW-18204): metadata doesn't matter by default
+  # other_metadata <- list_of(field("item", int32(), # nolint
+  #   metadata = list(hello="world"))) # nolint
+  # expect_equal(x, other_metadata) # nolint
+  # expect_false(x$Equals(other_metadata, check_metadata = TRUE)) # nolint

Review Comment:
   I think this would be more effective as a `reprex()` in ARROW-18204 rather than commented-out code here.



##########
r/tests/testthat/test-data-type.R:
##########
@@ -388,6 +402,20 @@ test_that("map type works as expected", {
   # we can make this comparison:
   # expect_equal(x$value_type, struct(key = x$key_field, value = x$item_field)) # nolint
   expect_false(x$keys_sorted)
+
+  # nullability matters in comparison
+  expect_false(x$Equals(map_of(int32(), field("value", utf8(), nullable = FALSE))))
+
+  # field names don't matter by default
+  other_name <- map_of(int32(), field("other", utf8()))
+  expect_equal(x, other_name)
+  expect_false(x$Equals(other_name, check_internal_field_names = TRUE))
+
+  # TODO(ARROW-18204): metadata doesn't matter by default
+  # other_metadata <- map_of(int32(), # nolint
+  #   field("value", int32(), metadata = list(hello="world"))) # nolint
+  # expect_equal(x, other_metadata) # nolint
+  # expect_false(x$Equals(other_metadata, check_metadata = TRUE)) # nolint

Review Comment:
   Again, I would put this in the Jira rather than commented out code here.



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