From f35f7a9d7109e656f3aaf68638a3e6b4d085d2ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kirill=20M=C3=BCller?= Date: Sun, 4 Feb 2024 16:56:26 +0100 Subject: [PATCH 1/4] Test that `summarize()` strips off the subclass --- tests/testthat/test-summarise.R | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/testthat/test-summarise.R b/tests/testthat/test-summarise.R index eec046542a..35d2404c27 100644 --- a/tests/testthat/test-summarise.R +++ b/tests/testthat/test-summarise.R @@ -75,6 +75,20 @@ test_that("preserved class, but not attributes", { expect_null(attr(out, "res")) }) +test_that("strips off subclass", { + df <- new_data_frame(list(a = 1), class = "myclass") + out <- df %>% summarise(n = n()) + expect_s3_class(out, "data.frame", exact = TRUE) + out <- df %>% summarise(.by = a, n = n()) + expect_s3_class(out, "data.frame", exact = TRUE) + + df <- new_tibble(list(a = 1), class = "myclass") + out <- df %>% summarise(n = n()) + expect_s3_class(out, class(tibble()), exact = TRUE) + out <- df %>% summarise(.by = a, n = n()) + expect_s3_class(out, class(tibble()), exact = TRUE) +}) + test_that("works with unquoted values", { df <- tibble(g = c(1, 1, 2, 2, 2), x = 1:5) expect_equal(summarise(df, out = !!1), tibble(out = 1)) From e74e5190f01a556974bec7f98628c280438de285 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kirill=20M=C3=BCller?= Date: Tue, 13 Feb 2024 22:31:19 +0100 Subject: [PATCH 2/4] Align snapshot tests (#6987) * Align snapshot tests * Bump rlang and cli for snapshot tests --------- Co-authored-by: Davis Vaughan --- DESCRIPTION | 4 ++-- tests/testthat/_snaps/context.md | 2 +- tests/testthat/_snaps/summarise.md | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 35d2ebcfa0..1c8de59242 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -22,7 +22,7 @@ BugReports: https://github.com/tidyverse/dplyr/issues Depends: R (>= 3.5.0) Imports: - cli (>= 3.4.0), + cli (>= 3.6.2), generics, glue (>= 1.3.2), lifecycle (>= 1.0.3), @@ -30,7 +30,7 @@ Imports: methods, pillar (>= 1.9.0), R6, - rlang (>= 1.1.0), + rlang (>= 1.1.3), tibble (>= 3.2.0), tidyselect (>= 1.2.0), utils, diff --git a/tests/testthat/_snaps/context.md b/tests/testthat/_snaps/context.md index a0b60d1fd4..08b03fb5be 100644 --- a/tests/testthat/_snaps/context.md +++ b/tests/testthat/_snaps/context.md @@ -40,5 +40,5 @@ Code group_labels_details(c(a = 1, b = 2)) Output - [1] "`a = 1`, `b = 2`" + [1] "`a = 1` and `b = 2`" diff --git a/tests/testthat/_snaps/summarise.md b/tests/testthat/_snaps/summarise.md index 318edcb751..3f80a60a5a 100644 --- a/tests/testthat/_snaps/summarise.md +++ b/tests/testthat/_snaps/summarise.md @@ -124,7 +124,7 @@ Error in `summarise()`: i In argument: `a = rlang::env(a = 1)`. - i In group 1: `x = 1`, `y = 1`. + i In group 1: `x = 1` and `y = 1`. Caused by error: ! `a` must be a vector, not an environment. Code From 3d6aa76acffb23996d5df2f54d09c9f66a846526 Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Tue, 13 Feb 2024 16:56:25 -0500 Subject: [PATCH 3/4] Tweak the test a little --- tests/testthat/test-summarise.R | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/tests/testthat/test-summarise.R b/tests/testthat/test-summarise.R index 35d2404c27..2a0db3b23f 100644 --- a/tests/testthat/test-summarise.R +++ b/tests/testthat/test-summarise.R @@ -60,22 +60,23 @@ test_that("no expressions yields grouping data", { expect_equal(summarise(gf, !!!list()), tibble(x = 1:2)) }) -test_that("preserved class, but not attributes", { +test_that("doesn't preserve attributes", { df <- structure( data.frame(x = 1:10, g1 = rep(1:2, each = 5), g2 = rep(1:5, 2)), meta = "this is important" ) out <- df %>% summarise(n = n()) - expect_s3_class(out, "data.frame", exact = TRUE) expect_null(attr(out, "res")) out <- df %>% group_by(g1) %>% summarise(n = n()) - # expect_s3_class(out, "data.frame", exact = TRUE) expect_null(attr(out, "res")) }) test_that("strips off subclass", { + # We consider the data frame returned by `summarise()` to be + # "fundamentally a new data frame" + df <- new_data_frame(list(a = 1), class = "myclass") out <- df %>% summarise(n = n()) expect_s3_class(out, "data.frame", exact = TRUE) @@ -87,6 +88,14 @@ test_that("strips off subclass", { expect_s3_class(out, class(tibble()), exact = TRUE) out <- df %>% summarise(.by = a, n = n()) expect_s3_class(out, class(tibble()), exact = TRUE) + + gdf <- group_by(tibble(a = 1), a) + df <- gdf + class(df) <- c("myclass", class(gdf)) + out <- df %>% summarise(n = n(), .groups = "drop") + expect_s3_class(out, class(tibble()), exact = TRUE) + out <- df %>% summarise(n = n(), .groups = "keep") + expect_s3_class(out, class(gdf), exact = TRUE) }) test_that("works with unquoted values", { From 408da4fcbeec057c41453791e4f06b4f49d6a579 Mon Sep 17 00:00:00 2001 From: Davis Vaughan Date: Tue, 13 Feb 2024 17:03:03 -0500 Subject: [PATCH 4/4] Mention `summarise()` / `reframe()` behavior in extension docs --- R/generics.R | 4 +++- man/dplyr_extending.Rd | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/R/generics.R b/R/generics.R index a9155f1a44..cfb440e838 100644 --- a/R/generics.R +++ b/R/generics.R @@ -63,7 +63,9 @@ #' #' * `summarise()` and `reframe()` work similarly to `mutate()` but the data #' modified by `dplyr_col_modify()` comes from `group_data()` or is built -#' from `.by`. +#' from `.by`. Note that this means that the data frames returned by +#' `summarise()` and `reframe()` are fundamentally new data frames, and +#' will not retain any custom subclasses or attributes. #' #' * `select()` uses 1d `[` to select columns, then `names<-` to rename them. #' `rename()` just uses `names<-`. `relocate()` just uses 1d `[`. diff --git a/man/dplyr_extending.Rd b/man/dplyr_extending.Rd index 72d5106e6d..0aec418efe 100644 --- a/man/dplyr_extending.Rd +++ b/man/dplyr_extending.Rd @@ -82,7 +82,9 @@ It also uses 1d \code{[} to implement \code{.keep}, and will call \code{relocate either \code{.before} or \code{.after} are supplied. \item \code{summarise()} and \code{reframe()} work similarly to \code{mutate()} but the data modified by \code{dplyr_col_modify()} comes from \code{group_data()} or is built -from \code{.by}. +from \code{.by}. Note that this means that the data frames returned by +\code{summarise()} and \code{reframe()} are fundamentally new data frames, and +will not retain any custom subclasses or attributes. \item \code{select()} uses 1d \code{[} to select columns, then \verb{names<-} to rename them. \code{rename()} just uses \verb{names<-}. \code{relocate()} just uses 1d \code{[}. \item \code{inner_join()}, \code{left_join()}, \code{right_join()}, and \code{full_join()}