thisisnic commented on a change in pull request #11175:
URL: https://github.com/apache/arrow/pull/11175#discussion_r712525803
##########
File path: r/tests/testthat/test-dplyr-group-by.R
##########
@@ -62,9 +62,23 @@ test_that("ungroup", {
select(int, chr) %>%
ungroup() %>%
filter(int > 5) %>%
- summarize(min_int = min(int)),
- tbl,
- warning = TRUE
+ collect(),
+ tbl
+ )
+
+ # to confirm that the above expectation is actually testing what we think
it's
+ # testing, verify that expect_dplyr_equal() distinguishes between grouped and
+ # ungrouped tibbles
+ expect_error(
+ expect_dplyr_equal(
+ input %>%
+ group_by(chr) %>%
+ select(int, chr) %>%
+ (function(x) if (inherits(x, "tbl_df")) ungroup(x) else x) %>%
+ filter(int > 5) %>%
+ collect(),
+ tbl
+ )
Review comment:
I'm really not sure about this as I can see compelling arguments both
ways for including or not including this. I'm a bit unsure as it feels like
it's testing `expect_dplyr_equal()` and not `group_by()`, and given we don't
have plans to revert the updated (attribute-testing) behaviour of
`expect_dplyr_equal()` I don't know if it's strictly necessary. On the other
hand, if something somehow goes wrong with that function, this would catch it.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
