Style and fix&improve some .new_col_name validation

Author: brookslogan

R/grouped_epi_archive.R

@@ -284,8 +284,19 @@ epix_slide.grouped_epi_archive <- function(
   validate_slide_window_arg(.before, .x$private$ungrouped$time_type)
 
   checkmate::assert_string(.new_col_name, null.ok = TRUE)
-  if (identical(.new_col_name, "time_value")) {
-    cli_abort('`.new_col_name` must not be `"version"`; `epix_slide()` uses that column name to attach which of the `.versions` is associated with each slide computation') # nolint: line_length_linter
+  if (!is.null(.new_col_name)) {
+    if (.new_col_name %in% x$private$vars) {
+      cli_abort(c("`new_col_name` must not be one of the grouping column name(s);
+                   `epix_slide()` uses these column name(s) to label what group
+                   each slide computation came from.",
+        "i" = "{cli::qty(length(x$private$vars))} grouping column name{?s}
+                         {?was/were} {format_chr_with_quotes(x$private$vars)}",
+        "x" = "`new_col_name` was {format_chr_with_quotes(new_col_name)}"
+      ))
+    }
+    if (identical(.new_col_name, "version")) {
+      cli_abort('`.new_col_name` must not be `"version"`; `epix_slide()` uses that column name to attach the element of `.versions` associated with each slide computation') # nolint: line_length_linter
+    }
   }
 
   assert_logical(.all_versions, len = 1L)

R/slide.R

@@ -174,15 +174,16 @@ 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);
+      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)}"))
+        "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
+      cli_abort('`.new_col_name` must not be `"time_value"`; `epi_slide()` uses that column name to attach the element of `.ref_time_values` associated with each slide computation') # nolint: line_length_linter
     }
   }
 

tests/testthat/test-methods-epi_archive.R

@@ -123,6 +123,8 @@ test_that("epix_truncate_version_after returns the same groups as input grouped_
 })
 
 test_that("group_vars works as expected", {
-  expect_equal(ea2_data %>% as_epi_archive() %>% group_by(geo_value) %>% group_vars(),
-               "geo_value")
+  expect_equal(
+    ea2_data %>% as_epi_archive() %>% group_by(geo_value) %>% group_vars(),
+    "geo_value"
+  )
 })