WIP Add docs for de-dupe approach, part of the required validation

Forbidding `new_col_name` being among the labeling columns addresses some dedupe
cases where deduping would always lead to failure except for
completely-redundant computations (that only output computation labels rather
than and actual computation).

- This might not be complete in a edge case where `"slide_value"` is a grouping
variable. (E.g., from using a slide to assign a categorical trend, then doing a
grouped slide based on the trend.)

This is definitely only part of the dedupe handling.  Unpacked-column outputs
need to actually be de-duped.

Also, fix incorrect documentation for time_value filter for .all_versions = TRUE
while rebasing on other slide updates.

Author: brookslogan

R/methods-epi_archive.R

@@ -650,15 +650,18 @@ epix_detailed_restricted_mutate <- function(.data, ...) {
 #'   set to a regularly-spaced sequence of values set to cover the range of
 #'   `version`s in the `DT` plus the `versions_end`; the spacing of values will
 #'   be guessed (using the GCD of the skips between values).
-#' @param .new_col_name String indicating the name of the new column that will
-#'   contain the derivative values. The default is "slide_value" unless your
-#'   slide computations output data frames, in which case they will be unpacked
-#'   into the constituent columns and those names used. Note that setting
-#'   `.new_col_name` equal to an existing column name will overwrite this column.
+#' @param .new_col_name Either `NULL` or a string indicating the name of the new
+#'   column that will contain the derived values. The default, `NULL`, will use
+#'   the name "slide_value" unless your slide computations output data frames,
+#'   in which case they will be unpacked into the constituent columns and those
+#'   names used. If the resulting column name(s) overlap with the column names
+#'   used for labeling the computations, which are `group_vars(x)` and
+#'   `"version"`, then the values for these columns must be identical to the
+#'   labels we assign.
 #' @param .all_versions (Not the same as `.all_rows` parameter of `epi_slide`.) If
-#'   TRUE, then `.f` will be passed the version history (all
-#'   `version <= .ref_time_value`) for rows having `time_value` between
-#'   `.ref_time_value - before` and `.ref_time_value`. Otherwise, `.f` will be
+#'   `.all_versions = TRUE`, then `.f` will be passed the version history (all
+#'   `version <= .ref_time_value`) for rows having `time_value` of at least
+#'   `.version - before`. Otherwise, `.f` will be
 #'   passed only the most recent `version` for every unique `time_value`.
 #'   Default is `FALSE`.
 #' @return A tibble whose columns are: the grouping variables, `time_value`,

R/slide.R

@@ -27,7 +27,9 @@
 #'   and can also refer to `.x`, `.group_key`, and `.ref_time_value`. See
 #'   details.
 #' @param .new_col_name String indicating the name of the new column that will
-#'   contain the derivative values. Default is "slide_value"; note that setting
+#'   contain the derivative values. The default is "slide_value" unless your
+#'   slide computations output data frames, in which case they will be unpacked
+#'   into the constituent columns and those names used. Note that setting
 #'   `new_col_name` equal to an existing column name will overwrite this column.
 #'
 #' @template basic-slide-details
@@ -169,6 +171,21 @@ epi_slide <- function(
     }
   }
 
+  checkmate::assert_string(new_col_name, null.ok = TRUE)
+  if (!is.null(new_col_name)) {
+    if (new_col_name %in% group_vars(x)) {
+      cli_abort(c("`new_col_name` must not be one of the grouping column name(s);
+                   `epi_slide()` uses these column name(s) to label what group
+                   each slide computation came from.",
+                  "i" = "{cli::qty(length(group_vars(x)))} grouping column name{?s}
+                         {?was/were} {format_chr_with_quotes(group_vars(x))}",
+                  "x" = "`new_col_name` was {format_chr_with_quotes(new_col_name)}"))
+    }
+    if (identical(new_col_name, "time_value")) {
+      cli_abort('`new_col_name` must not be `"time_value"`; `epi_slide()` uses that column name to attach the `ref_time_value` associated with each slide computation') # nolint: line_length_linter
+    }
+  }
+
   # Arrange by increasing time_value
   x <- arrange(.x, .data$time_value)
 

R/utils.R

@@ -97,6 +97,28 @@ format_class_vec <- function(class_vec) {
   paste(collapse = "", deparse(class_vec))
 }
 
+#' Format a character vector as a string via deparsing/quoting each
+#'
+#' @param x `chr`; e.g., `colnames` of some data frame
+#' @param empty string; what should be output if `x` is of length 0?
+#' @return string
+format_chr_with_quotes <- function(x, empty = "*none*") {
+  if (length(x) == 0L) {
+    empty
+  } else {
+    # Deparse to get quoted + escape-sequenced versions of varnames; collapse to
+    # single line (assuming no newlines in `x`). Though if we hand this to cli
+    # it may insert them (even in middle of quotes) while wrapping lines.
+    deparsed_collapsed <- paste(collapse = "", deparse(x))
+    if (length(x) == 1L) {
+      deparsed_collapsed
+    } else {
+      # remove surrounding `c()`:
+      substr(deparsed_collapsed, 3L, nchar(deparsed_collapsed) - 1L)
+    }
+  }
+}
+
 #' Assert that a sliding computation function takes enough args
 #'
 #' @param f Function; specifies a computation to slide over an `epi_df` or