nealrichardson commented on a change in pull request #8150:
URL: https://github.com/apache/arrow/pull/8150#discussion_r487119708



##########
File path: r/tests/testthat/test-dataset.R
##########
@@ -890,6 +890,7 @@ test_that("Dataset writing: from data.frame", {
       select(string = chr, integer = int) %>%
       filter(integer > 6 & integer < 11) %>%
       collect() %>%
+      ungroup() %>%

Review comment:
       This isn't desirable because for `write_dataset()`, `group_by` is used 
to convey which columns to partition the dataset on. So I think there needs to 
be an `ungroup()` somewhere around L71 in write-dataset.R

##########
File path: r/tests/testthat/test-metadata.R
##########
@@ -124,5 +125,11 @@ test_that("haven types roundtrip via feather", {
 test_that("Date/time type roundtrip", {
   rb <- record_batch(example_with_times)
   expect_is(rb$schema$posixlt$type, "StructType")
-  expect_identical(as.data.frame(rb), example_with_times)
+  expect_equal(as.data.frame(rb), example_with_times)
+})
+
+test_that("metadata keeps attribute of top level data frame", {
+  df <- structure(data.frame(x = 1, y = 2), foo = "bar")
+  tab <- Table$create(df)
+  expect_equal(attr(as.data.frame(tab), "foo"), "bar")

Review comment:
       why not `expect_identical(as.data.frame(tab), df)`?




----------------------------------------------------------------
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:
[email protected]


Reply via email to