From 4f01b5541d3cf29775b4bb073cc958e35cc0dcd9 Mon Sep 17 00:00:00 2001
From: "Logan C. Brooks" <lcbrooks@andrew.cmu.edu>
Date: Wed, 26 Apr 2023 19:51:29 -0700
Subject: [PATCH 1/2] Round out grouped `epi_df` ops using new `grouped_edf`
 class

Motivation:
* In the current `c("epi_df", "grouped_df", "tbl_df", "tbl", "data.frame")`
  approach without many `epi_df` methods, there are several `grouped_df` methods
  that will drop the `epi_df` class.
* `tbl_sum.grouped_df` expects `grouped_df` to be in front / a subclass of the
  "base" class for ungrouped data.

So move to make grouped `epi_df`s have class vector `c("grouped_edf",
"grouped_df", "epi_df", "tbl_df", "tbl", "data.frame")`, and implement a handful
of methods for `grouped_edf` to try to keep around `epi_df`-ness and
`grouped_edf`-ness when appropriate. Replace `reclass` with multiple similar
functions to facilitate this change.
---
 DESCRIPTION                                   |   4 +-
 NAMESPACE                                     |  22 ++-
 R/grouped_edf.R                               | 160 ++++++++++++++++++
 R/methods-epi_df.R                            | 117 +++++++------
 man/as_inheriting.Rd                          |  30 ++++
 ...print.epi_df.Rd => basic_s3_for_epi_df.Rd} |  30 ++--
 tests/testthat/test-epi_df.R                  |  38 ++++-
 7 files changed, 326 insertions(+), 75 deletions(-)
 create mode 100644 R/grouped_edf.R
 create mode 100644 man/as_inheriting.Rd
 rename man/{print.epi_df.Rd => basic_s3_for_epi_df.Rd} (58%)

diff --git a/DESCRIPTION b/DESCRIPTION
index 8a06e6f5..7cc034a5 100755
--- a/DESCRIPTION
+++ b/DESCRIPTION
@@ -31,6 +31,7 @@ Imports:
     lifecycle (>= 1.0.1),
     lubridate,
     magrittr,
+    pillar,
     purrr,
     R6,
     rlang,
@@ -61,7 +62,7 @@ Config/testthat/edition: 3
 Encoding: UTF-8
 LazyData: true
 Roxygen: list(markdown = TRUE)
-RoxygenNote: 7.2.1
+RoxygenNote: 7.2.3
 Depends: 
     R (>= 2.10)
 URL: https://cmu-delphi.github.io/epiprocess/
@@ -71,6 +72,7 @@ Collate:
     'data.R'
     'epi_df.R'
     'epiprocess.R'
+    'grouped_edf.R'
     'methods-epi_archive.R'
     'grouped_epi_archive.R'
     'growth_rate.R'
diff --git a/NAMESPACE b/NAMESPACE
index 10847e6c..05bf2d5d 100644
--- a/NAMESPACE
+++ b/NAMESPACE
@@ -1,30 +1,44 @@
 # Generated by roxygen2: do not edit by hand
 
+S3method("$<-",grouped_edf)
 S3method("[",epi_df)
+S3method("[",grouped_edf)
+S3method("[<-",grouped_edf)
+S3method("[[<-",grouped_edf)
 S3method("names<-",epi_df)
+S3method("names<-",grouped_edf)
 S3method(as_epi_df,data.frame)
 S3method(as_epi_df,epi_df)
 S3method(as_epi_df,tbl_df)
 S3method(as_epi_df,tbl_ts)
 S3method(as_tsibble,epi_df)
+S3method(do,grouped_edf)
 S3method(dplyr_col_modify,col_modify_recorder_df)
 S3method(dplyr_col_modify,epi_df)
+S3method(dplyr_col_modify,grouped_edf)
 S3method(dplyr_reconstruct,epi_df)
+S3method(dplyr_reconstruct,grouped_edf)
 S3method(dplyr_row_slice,epi_df)
+S3method(dplyr_row_slice,grouped_edf)
 S3method(epix_truncate_versions_after,epi_archive)
 S3method(epix_truncate_versions_after,grouped_epi_archive)
 S3method(group_by,epi_archive)
 S3method(group_by,epi_df)
 S3method(group_by,grouped_epi_archive)
 S3method(group_by_drop_default,grouped_epi_archive)
+S3method(group_data,grouped_edf)
+S3method(group_modify,grouped_edf)
+S3method(group_trim,grouped_edf)
 S3method(groups,grouped_epi_archive)
 S3method(next_after,Date)
 S3method(next_after,integer)
-S3method(print,epi_df)
+S3method(summarise,grouped_edf)
 S3method(summary,epi_df)
-S3method(ungroup,epi_df)
+S3method(tbl_sum,epi_df)
+S3method(ungroup,grouped_edf)
 S3method(ungroup,grouped_epi_archive)
 S3method(unnest,epi_df)
+S3method(vec_restore,grouped_edf)
 export("%>%")
 export(archive_cases_dv_subset)
 export(arrange)
@@ -67,12 +81,14 @@ importFrom(data.table,key)
 importFrom(data.table,set)
 importFrom(data.table,setkeyv)
 importFrom(dplyr,arrange)
+importFrom(dplyr,do)
 importFrom(dplyr,dplyr_col_modify)
 importFrom(dplyr,dplyr_reconstruct)
 importFrom(dplyr,dplyr_row_slice)
 importFrom(dplyr,filter)
 importFrom(dplyr,group_by)
 importFrom(dplyr,group_by_drop_default)
+importFrom(dplyr,group_data)
 importFrom(dplyr,group_modify)
 importFrom(dplyr,groups)
 importFrom(dplyr,mutate)
@@ -84,6 +100,7 @@ importFrom(dplyr,ungroup)
 importFrom(lubridate,days)
 importFrom(lubridate,weeks)
 importFrom(magrittr,"%>%")
+importFrom(pillar,tbl_sum)
 importFrom(rlang,"!!!")
 importFrom(rlang,"!!")
 importFrom(rlang,.data)
@@ -101,3 +118,4 @@ importFrom(tidyr,unnest)
 importFrom(tidyselect,eval_select)
 importFrom(tidyselect,starts_with)
 importFrom(tsibble,as_tsibble)
+importFrom(vctrs,vec_restore)
diff --git a/R/grouped_edf.R b/R/grouped_edf.R
new file mode 100644
index 00000000..6da1af03
--- /dev/null
+++ b/R/grouped_edf.R
@@ -0,0 +1,160 @@
+#' Simple reclass function for `grouped_edf`s; typically favor `maybe` variant
+#'
+#' Ensures that result has class `grouped_edf`, but due to `dplyr`'s treatment
+#' of trivial groupings, we may not want to always output `grouped_edf`
+#' "when grouping".
+#'
+#' @param x Object to reclass to `grouped_edf`
+#' @param potential_subsubclasses As in [`as_inheriting`]
+#' @return A `grouped_edf`
+#'
+#' @noRd
+reclass_definitely_grouped_edf = function(x, metadata, potential_subsubclasses=character(0L)) {
+  as_inheriting(x, "grouped_edf", potential_subsubclasses)
+}
+
+#' Simple reclass function for `grouped_edf`s, unless not a `grouped_df`&`epi_df`
+#'
+#' Typically favor this over `reclass_definitely_grouped_edf`, as "grouping"
+#' operations won't actually produce `grouped_df`s if it's a trival grouping,
+#' and we want to add/drop the `grouped_edf` class in the same situations as
+#' `grouped_df` for `epi_df`s. This is provided that we are indeed given an
+#' `epi_df`; if instead we're given, e.g., something from
+#' `dplyr_reconstruct.epi_df` that might have decayed to a non-`epi_df`, we
+#' should not form a `grouped_edf`.
+#'
+#' @param x Object to reclass to `grouped_edf` if it's a `grouped_df`
+#' @param potential_subsubclasses As in [`as_inheriting`]
+#' @return A `grouped_edf` if `x` was a `grouped_df`; otherwise just `x`
+#'
+#' @noRd
+reclass_maybe_grouped_edf = function(x, metadata, potential_subsubclasses=character(0L)) {
+  if (inherits(x, "grouped_df") && inherits(x, "epi_df")) {
+    reclass_definitely_grouped_edf(x, metadata, potential_subsubclasses)
+  } else {
+    x
+  }
+}
+
+#' @rdname basic_s3_for_epi_df
+#' @aliases grouped_edf
+#' @export
+group_by.epi_df = function(.data, ...) {
+  metadata = attr(.data, "metadata")
+  result = NextMethod()
+  result <- reclass_epi_df(result, metadata, "grouped_df")
+  result <- reclass_maybe_grouped_edf(result)
+  result
+  # reclass_epi_df(.data, metadata, "grouped_df")
+  # reclass_epi_df(.data, metadata)
+}
+
+#' @importFrom dplyr group_data
+#' @rdname basic_s3_for_epi_df
+#' @export
+group_data.grouped_edf = function(.data) {
+  metadata = attr(.data, "metadata")
+  reclass_epi_df(NextMethod(), metadata)
+}
+
+#' @importFrom dplyr dplyr_reconstruct
+#' @export
+#' @noRd
+dplyr_reconstruct.grouped_edf = function(data, template) {
+  # dplyr_reconstruct.grouped_df doesn't naturally call
+  # dplyr_reconstruct.epi_df; do that first. We can't just call
+  # dplyr_reconstruct.epi_df as it relies on NextMethod(), which relies on
+  # actual S3 dispatch; get that S3 dispatch by calling the generic using a
+  # tweaked version of `template` (the S3 dispatch arg for `dplyr_reconstruct`).
+  template_class = class(template)
+  edf_template = `class<-`(
+    template,
+    template_class[! template_class %in% c("grouped_edf", "grouped_df")]
+  )
+  data <- dplyr_reconstruct(data, edf_template)
+  # `data` here is either `epi_df` or decayed
+  result = NextMethod()
+  # Assume that dplyr_reconstruct.grouped_df hasn't done anything that would
+  # cause an epi_df to need to decay (but won't output an `epi_df` class ever).
+  # So just see if `data` was `epi_df` or decayed and mirror that by adding
+  # `epi_df` class / not.
+  if (inherits(data, "epi_df")) {
+    metadata = attr(data, "metadata")
+    result <- reclass_epi_df(result, metadata, "grouped_df")
+  }
+  result <- reclass_maybe_grouped_edf(result)
+  result
+}
+
+#' @importFrom vctrs vec_restore
+#' @export
+#' @noRd
+vec_restore.grouped_edf = function(x, to, ...) {
+  dplyr_reconstruct(x, to)
+}
+
+#' @rdname basic_s3_for_epi_df
+#' @export
+ungroup.grouped_edf = function(x, ...) {
+  metadata = attr(x, "metadata")
+  result = NextMethod()
+  result <- reclass_epi_df(result, metadata, "grouped_df")
+  result <- reclass_maybe_grouped_edf(result)
+  result
+}
+
+#' @export
+`[.grouped_edf` = function(x, i, j, drop = FALSE) {
+  dplyr_reconstruct(NextMethod(), x)
+}
+
+#' @export
+`[[<-.grouped_edf` = function(x, ..., value) {
+  dplyr_reconstruct(NextMethod(), x)
+}
+
+#' @export
+`[<-.grouped_edf` = function(x, i, j, ..., value) {
+  dplyr_reconstruct(NextMethod(), x)
+}
+
+#' @export
+`$<-.grouped_edf` = function(x, name, ..., value) {
+  dplyr_reconstruct(NextMethod(), x)
+}
+
+#' @importFrom dplyr do
+#' @export
+do.grouped_edf = function(.data, ...) {
+  dplyr_reconstruct(NextMethod(), .data)
+}
+
+#' @export
+dplyr_col_modify.grouped_edf = function(data, cols) {
+  dplyr_reconstruct(NextMethod(), data)
+}
+
+#' @export
+dplyr_row_slice.grouped_edf = function(data, i, ..., preserve = FALSE) {
+  dplyr_reconstruct(NextMethod(), data)
+}
+
+#' @export
+group_modify.grouped_edf = function(.data, .f, ..., .keep = FALSE, keep = deprecated()) {
+  dplyr_reconstruct(NextMethod(), .data)
+}
+
+#' @export
+group_trim.grouped_edf = function(.tbl, .drop = group_by_drop_default(.tbl)) {
+  dplyr_reconstruct(NextMethod(), .tbl)
+}
+
+#' @export
+`names<-.grouped_edf` = function(x, value) {
+  dplyr_reconstruct(NextMethod(), x)
+}
+
+#' @export
+summarise.grouped_edf = function(.data, ..., .by = NULL, .groups = NULL) {
+  dplyr_reconstruct(NextMethod(), .data)
+}
diff --git a/R/methods-epi_df.R b/R/methods-epi_df.R
index 503b8add..95f0e40d 100644
--- a/R/methods-epi_df.R
+++ b/R/methods-epi_df.R
@@ -17,23 +17,16 @@ as_tsibble.epi_df = function(x, key, ...) {
                     ...))
 }
 
-#' Base S3 methods for an `epi_df` object
-#'
-#' Print and summary functions for an `epi_df` object.
-#'
-#' @param x The `epi_df` object.
-#' @param ... Additional arguments passed to methods.
-#'
-#' @method print epi_df
+#' Basic S3 methods for an `epi_df` object
+#' @name basic_s3_for_epi_df
+NULL
+
+#' @importFrom pillar tbl_sum
+#' @rdname basic_s3_for_epi_df
 #' @export
-print.epi_df = function(x, ...) {
-  cat("An `epi_df` object,", prettyNum(nrow(x),","), "x",
-      prettyNum(ncol(x),","), "with metadata:\n")
-  cat(sprintf("* %-9s = %s\n", "geo_type", attributes(x)$metadata$geo_type))
-  cat(sprintf("* %-9s = %s\n", "time_type", attributes(x)$metadata$time_type))
-  cat(sprintf("* %-9s = %s\n", "as_of", attributes(x)$metadata$as_of))
-  cat("\n")
-  NextMethod()
+tbl_sum.epi_df = function(x, ...) {
+  c("An epi_df" = pillar::dim_desc(x),
+    purrr::map_chr(attr(x, "metadata"), toString))
 }
 
 #' Summarize `epi_df` object
@@ -46,7 +39,7 @@ print.epi_df = function(x, ...) {
 #'   Currently unused.
 #'
 #' @method summary epi_df
-#' @rdname print.epi_df
+#' @rdname basic_s3_for_epi_df
 #' @importFrom rlang .data
 #' @importFrom stats median
 #' @export
@@ -64,7 +57,7 @@ summary.epi_df = function(object, ...) {
                          dplyr::summarize(mean(.data$num)))))
 }
 
-#' Drop any `epi_df` metadata and class on a data frame
+#' Drop any `epi_df` metadata and `epi_df`/`grouped_edf` class on a data frame
 #'
 #' Useful in implementing `?dplyr_extending` when manipulations cause invariants
 #' of `epi_df`s to be violated and we need to return some other class. Note that
@@ -72,13 +65,13 @@ summary.epi_df = function(object, ...) {
 #' associated attributes, if present).
 #'
 #' @param x an `epi_df` or other data frame
-#' @return `x` with any metadata dropped and the `"epi_df"` class, if previously
-#'   present, dropped
+#' @return `x` with any metadata dropped and the `"epi_df"` and `"grouped_edf"`
+#'   classes, if previously present, dropped
 #'
 #' @noRd
 decay_epi_df = function(x) {
   attributes(x)$metadata <- NULL
-  class(x) <- class(x)[class(x) != "epi_df"]
+  class(x) <- class(x)[! class(x) %in% c("grouped_edf", "epi_df")]
   x
 }
 
@@ -126,7 +119,7 @@ dplyr_reconstruct.epi_df = function(data, template) {
     return(decay_epi_df(res))
   }
   
-  res <- reclass(res, attr(template, "metadata"))
+  res <- reclass_epi_df(res, attr(template, "metadata"))
 
   # XXX we may want verify the `geo_type` and `time_type` here. If it's
   # significant overhead, we may also want to keep this less strict version
@@ -166,46 +159,60 @@ dplyr_row_slice.epi_df = function(data, i, ...) {
   old_other_keys = attributes(x)$metadata$other_keys
   result = NextMethod()
   attributes(x)$metadata$other_keys <- value[match(old_other_keys, old_names)]
-  dplyr::dplyr_reconstruct(result, result)
-}
-
-#' @method group_by epi_df
-#' @rdname print.epi_df
-#' @export
-group_by.epi_df = function(.data, ...) {
-  metadata = attributes(.data)$metadata
-  .data = NextMethod()
-  reclass(.data, metadata)
-}
-
-#' @method ungroup epi_df
-#' @rdname print.epi_df
-#' @export
-ungroup.epi_df = function(x, ...) {
-  metadata = attributes(x)$metadata
-  x = NextMethod()
-  reclass(x, metadata)
-}
-
-#' @method unnest epi_df
-#' @rdname print.epi_df
-#' @param data The `epi_df` object.
-#' @export
-group_modify.epi_df = function(.data, .f, ..., .keep = FALSE) {
-  dplyr::dplyr_reconstruct(NextMethod(), .data)
+  dplyr::dplyr_reconstruct(result, x)
 }
 
 #' @method unnest epi_df
-#' @rdname print.epi_df
+#' @rdname basic_s3_for_epi_df
 #' @param data The `epi_df` object.
 #' @export
 unnest.epi_df = function(data, ...) {
   dplyr::dplyr_reconstruct(NextMethod(), data)
 }
 
-# Simple reclass function
-reclass = function(x, metadata) {
-  class(x) = unique(c("epi_df", class(x)))
-  attributes(x)$metadata = metadata
-  return(x)
+#' Reinterpretation of `x` as inheriting `subclass` if it doesn't already
+#'
+#' @param x an S3 object
+#' @param subclass string; S3 class name we want to ensure that the result will
+#'   [`base::inherit`]. We call this a "subclass" because we generally expect it
+#'   to be in the beginning or middle of a `class` vector, followed by some
+#'   other S3 class(es). If `x` already inherits this subclass, we return `x`
+#'   unchanged.
+#' @param potential_subsubclasses Optional; character vector: if `class(x)`
+#'   doesn't already include `subclass`, it should be added in the first
+#'   position after all of the `potential_subsubclasses` that appear in
+#'   `class(x)`. This lets us control the order of `subclass` relative to
+#'   "base"-like classes such as `"tbl_df"` and other wrapper classes like
+#'   `"grouped_df"`. Will be ignored if `subclass` is already in `class(x)`.
+#' @return object that inherits `subclass`
+as_inheriting = function(x, subclass, potential_subsubclasses=character(0L)) {
+  if(inherits(x, subclass)) {
+    return(x)
+  } else {
+    insertion_index =
+      max(0L, match(potential_subsubclasses, class(x)), na.rm = TRUE) + 1L
+    result = x
+    class(result) <- c(
+      class(x)[seq_len(insertion_index - 1L)],
+      subclass,
+      class(x)[insertion_index - 1L +
+                 seq_len(length(class(x)) - insertion_index + 1L)]
+    )
+    return(result)
+  }
+}
+
+#' Simple reclass function for `epi_df`
+#'
+#' @param x Object to reclass to `epi_df`
+#' @param metadata List; `epi_df` metadata to assign
+#' @param potential_subsubclasses As in [`as_inheriting`]
+#' @return An `epi_df`
+#'
+#' @noRd
+reclass_epi_df = function(x, metadata, potential_subsubclasses=character(0L)) {
+  result = as_inheriting(x, "epi_df", potential_subsubclasses)
+  attributes(result)$metadata = metadata
+  return(result)
 }
+
diff --git a/man/as_inheriting.Rd b/man/as_inheriting.Rd
new file mode 100644
index 00000000..ba69cabe
--- /dev/null
+++ b/man/as_inheriting.Rd
@@ -0,0 +1,30 @@
+% Generated by roxygen2: do not edit by hand
+% Please edit documentation in R/methods-epi_df.R
+\name{as_inheriting}
+\alias{as_inheriting}
+\title{Reinterpretation of \code{x} as inheriting \code{subclass} if it doesn't already}
+\usage{
+as_inheriting(x, subclass, potential_subsubclasses = character(0L))
+}
+\arguments{
+\item{x}{an S3 object}
+
+\item{subclass}{string; S3 class name we want to ensure that the result will
+\code{\link[base:inherit]{base::inherit}}. We call this a "subclass" because we generally expect it
+to be in the beginning or middle of a \code{class} vector, followed by some
+other S3 class(es). If \code{x} already inherits this subclass, we return \code{x}
+unchanged.}
+
+\item{potential_subsubclasses}{Optional; character vector: if \code{class(x)}
+doesn't already include \code{subclass}, it should be added in the first
+position after all of the \code{potential_subsubclasses} that appear in
+\code{class(x)}. This lets us control the order of \code{subclass} relative to
+"base"-like classes such as \code{"tbl_df"} and other wrapper classes like
+\code{"grouped_df"}. Will be ignored if \code{subclass} is already in \code{class(x)}.}
+}
+\value{
+object that inherits \code{subclass}
+}
+\description{
+Reinterpretation of \code{x} as inheriting \code{subclass} if it doesn't already
+}
diff --git a/man/print.epi_df.Rd b/man/basic_s3_for_epi_df.Rd
similarity index 58%
rename from man/print.epi_df.Rd
rename to man/basic_s3_for_epi_df.Rd
index 878e7f18..7903af97 100644
--- a/man/print.epi_df.Rd
+++ b/man/basic_s3_for_epi_df.Rd
@@ -1,29 +1,29 @@
 % Generated by roxygen2: do not edit by hand
-% Please edit documentation in R/methods-epi_df.R
-\name{print.epi_df}
-\alias{print.epi_df}
-\alias{summary.epi_df}
+% Please edit documentation in R/grouped_edf.R, R/methods-epi_df.R
+\name{group_by.epi_df}
 \alias{group_by.epi_df}
-\alias{ungroup.epi_df}
-\alias{group_modify.epi_df}
+\alias{grouped_edf}
+\alias{group_data.grouped_edf}
+\alias{ungroup.grouped_edf}
+\alias{basic_s3_for_epi_df}
+\alias{tbl_sum.epi_df}
+\alias{summary.epi_df}
 \alias{unnest.epi_df}
-\title{Base S3 methods for an \code{epi_df} object}
+\title{Basic S3 methods for an \code{epi_df} object}
 \usage{
-\method{print}{epi_df}(x, ...)
+\method{group_by}{epi_df}(.data, ...)
 
-\method{summary}{epi_df}(object, ...)
+\method{group_data}{grouped_edf}(.data)
 
-\method{group_by}{epi_df}(.data, ...)
+\method{ungroup}{grouped_edf}(x, ...)
 
-\method{ungroup}{epi_df}(x, ...)
+\method{tbl_sum}{epi_df}(x, ...)
 
-\method{unnest}{epi_df}(.data, .f, ..., .keep = FALSE)
+\method{summary}{epi_df}(object, ...)
 
 \method{unnest}{epi_df}(data, ...)
 }
 \arguments{
-\item{x}{The \code{epi_df} object.}
-
 \item{...}{Additional arguments, for compatibility with \code{summary()}.
 Currently unused.}
 
@@ -32,8 +32,6 @@ Currently unused.}
 \item{data}{The \code{epi_df} object.}
 }
 \description{
-Print and summary functions for an \code{epi_df} object.
-
 Prints a variety of summary statistics about the \code{epi_df} object, such as
 the time range included and geographic coverage.
 }
diff --git a/tests/testthat/test-epi_df.R b/tests/testthat/test-epi_df.R
index 10b0015e..b5bfaa5c 100644
--- a/tests/testthat/test-epi_df.R
+++ b/tests/testthat/test-epi_df.R
@@ -38,4 +38,40 @@ test_that("as_epi_df errors when additional_metadata is not a list", {
   expect_error(
     as_epi_df(ex_input, additional_metadata = c(other_keys = "state", "pol")), 
     "`additional_metadata` must be a list type.")
-})
\ No newline at end of file
+})
+
+# test_that("`reclass` `after_classes` works as intended", {
+#   edf = jhu_csse_county_level_subset
+#   tbl = tibble::as_tibble(edf)
+#   grouped_tbl = tbl %>% group_by(geo_value)
+#   metadata = attr(jhu_csse_county_level_subset, "metadata")
+#   # Simple default behavior:
+#   expect_identical(
+#     class(reclass(tbl, metadata)),
+#     c("epi_df", "tbl_df", "tbl", "data.frame")
+#   )
+#   # Behavior when we already have "epi_df" class:
+#   expect_identical(
+#     class(reclass(edf, metadata)),
+#     c("epi_df", "tbl_df", "tbl", "data.frame")
+#   )
+#   some_other_ordering = c("tbl_df", "epi_df", "tbl", "data.frame")
+#   expect_identical(
+#     class(reclass(`class<-`(edf, some_other_ordering), metadata)),
+#     some_other_ordering
+#   )
+#   # Controlling ordering moving from default to passing nondefault
+#   # `before_classes`:
+#   expect_identical(
+#     class(reclass(grouped_tbl, metadata)),
+#     c("epi_df", "grouped_df", "tbl_df", "tbl", "data.frame")
+#   )
+#   expect_identical(
+#     class(reclass(grouped_tbl, metadata, "grouped_df")),
+#     c("grouped_df", "epi_df", "tbl_df", "tbl", "data.frame")
+#   )
+#   expect_identical(
+#     class(reclass(tbl, metadata, "grouped_df")),
+#     c("epi_df", "tbl_df", "tbl", "data.frame")
+#   )
+# })

From f0135f1ae0da602b951b528d3701119d11f647d5 Mon Sep 17 00:00:00 2001
From: "Logan C. Brooks" <lcbrooks@andrew.cmu.edu>
Date: Mon, 1 May 2023 12:31:15 -0700
Subject: [PATCH 2/2] Fix missing `group_trim` import, adjust some comments

---
 NAMESPACE          | 3 ++-
 R/grouped_edf.R    | 4 +++-
 R/methods-epi_df.R | 3 +--
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/NAMESPACE b/NAMESPACE
index 05bf2d5d..f2337a71 100644
--- a/NAMESPACE
+++ b/NAMESPACE
@@ -32,7 +32,6 @@ S3method(group_trim,grouped_edf)
 S3method(groups,grouped_epi_archive)
 S3method(next_after,Date)
 S3method(next_after,integer)
-S3method(summarise,grouped_edf)
 S3method(summary,epi_df)
 S3method(tbl_sum,epi_df)
 S3method(ungroup,grouped_edf)
@@ -69,6 +68,7 @@ export(next_after)
 export(relocate)
 export(rename)
 export(slice)
+export(summarise.grouped_edf)
 export(ungroup)
 export(unnest)
 importFrom(R6,R6Class)
@@ -90,6 +90,7 @@ importFrom(dplyr,group_by)
 importFrom(dplyr,group_by_drop_default)
 importFrom(dplyr,group_data)
 importFrom(dplyr,group_modify)
+importFrom(dplyr,group_trim)
 importFrom(dplyr,groups)
 importFrom(dplyr,mutate)
 importFrom(dplyr,relocate)
diff --git a/R/grouped_edf.R b/R/grouped_edf.R
index 6da1af03..ce1a36c1 100644
--- a/R/grouped_edf.R
+++ b/R/grouped_edf.R
@@ -72,7 +72,8 @@ dplyr_reconstruct.grouped_edf = function(data, template) {
     template_class[! template_class %in% c("grouped_edf", "grouped_df")]
   )
   data <- dplyr_reconstruct(data, edf_template)
-  # `data` here is either `epi_df` or decayed
+  # `data` here is either `epi_df` or decayed. Now apply `grouped_df`
+  # reconstruction (remember S3 dispatch arg is `template`):
   result = NextMethod()
   # Assume that dplyr_reconstruct.grouped_df hasn't done anything that would
   # cause an epi_df to need to decay (but won't output an `epi_df` class ever).
@@ -144,6 +145,7 @@ group_modify.grouped_edf = function(.data, .f, ..., .keep = FALSE, keep = deprec
   dplyr_reconstruct(NextMethod(), .data)
 }
 
+#' @importFrom dplyr group_trim
 #' @export
 group_trim.grouped_edf = function(.tbl, .drop = group_by_drop_default(.tbl)) {
   dplyr_reconstruct(NextMethod(), .tbl)
diff --git a/R/methods-epi_df.R b/R/methods-epi_df.R
index 95f0e40d..7858a606 100644
--- a/R/methods-epi_df.R
+++ b/R/methods-epi_df.R
@@ -94,8 +94,6 @@ decay_epi_df = function(x) {
 #' @export
 #' @noRd
 dplyr_reconstruct.epi_df = function(data, template) {
-  # Start from a reconstruction for the backing S3 classes; this ensures that we
-  # keep any grouping that has been applied:
   res <- NextMethod()
   
   cn <- names(res)
@@ -167,6 +165,7 @@ dplyr_row_slice.epi_df = function(data, i, ...) {
 #' @param data The `epi_df` object.
 #' @export
 unnest.epi_df = function(data, ...) {
+  # XXX This should be updating `other_keys`, but isn't; issue #306.
   dplyr::dplyr_reconstruct(NextMethod(), data)
 }