jonkeane commented on a change in pull request #12073:
URL: https://github.com/apache/arrow/pull/12073#discussion_r778347037
##########
File path: r/tests/testthat/test-metadata.R
##########
@@ -367,3 +367,16 @@ test_that("grouped_df metadata is recorded (efficiently)",
{
expect_r6_class(grouped_tab, "Table")
expect_equal(grouped_tab$r_metadata$attributes$.group_vars, "a")
})
+
+test_that("grouped_df non-arrow metadata is preserved", {
+
+ simple_tbl <- tibble(a = 1:2, b = 3:4)
+ attr(simple_tbl, "other_metadata") <- "look I'm still here!"
Review comment:
`"look I'm still here!"`
😂
##########
File path: r/R/metadata.R
##########
@@ -115,7 +115,7 @@ apply_arrow_r_metadata <- function(x, r_metadata) {
remove_attributes <- function(x) {
removed_attributes <- character()
if (identical(class(x), c("tbl_df", "tbl", "data.frame"))) {
- removed_attributes <- c("class", "row.names", "names")
+ removed_attributes <- c("class", "row.names", "names", "groups")
Review comment:
It might be hard to find a definitive answer if this is ok, but this
will remove any `groups` attribute whether or not it is from dplyr grouping. It
is almost certainly ok, since dplyr _itself_ would overwrite this attribute if
something used it, but in this code path, we aren't any more checking
`inherits(x, "grouped_df")` before deleting that attribute.
Thoughts?
--
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-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
