eitsupi commented on code in PR #14160:
URL: https://github.com/apache/arrow/pull/14160#discussion_r975199513
##########
r/tests/testthat/test-dplyr-query.R:
##########
@@ -119,7 +119,7 @@ test_that("collect(as_data_frame=FALSE)", {
filter(int > 5) %>%
group_by(int) %>%
collect(as_data_frame = FALSE)
- expect_s3_class(b4, "arrow_dplyr_query")
Review Comment:
Thanks for your review.
> I think we need a test (and matching code change) that a Table with groups
in the `$metadata` prints that it is grouped. Currently, the print method does
not reveal that you have groupings, even though they get restored when you
`as.data.frame()`:
I agree that the print results need improvement.
But would it be better to create a separate ticket and do it there?
This occurs even in arrow 9.0.0, regardless of the behavior with `compute`.
``` r
mtcars |> dplyr::group_by(cyl) |> arrow::arrow_table()
#> Table
#> 32 rows x 11 columns
#> $mpg <double>
#> $cyl <double>
#> $disp <double>
#> $hp <double>
#> $drat <double>
#> $wt <double>
#> $qsec <double>
#> $vs <double>
#> $am <double>
#> $gear <double>
#> $carb <double>
#>
#> See $metadata for additional Schema metadata
```
<sup>Created on 2022-09-20 with [reprex
v2.0.2](https://reprex.tidyverse.org)</sup>
> This feature of reading groups from `$metadata$r$attributes$.group_vars`
was a bit of a hack and not something we're generally relying on, so I don't
think it's a simple change. Or at least, it's not something we can just rely
that the test suite already covers everything we care about.
Since `dplyr::group_vars` reads this field, isn't this function generally
used?
--
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
