refactor(epix_slide): before defaults to Inf, remove time_step

NAMESPACE

@@ -95,6 +95,7 @@ importFrom(checkmate,check_atomic)
 importFrom(checkmate,check_data_frame)
 importFrom(checkmate,check_names)
 importFrom(checkmate,expect_class)
+importFrom(checkmate,testInt)
 importFrom(checkmate,test_set_equal)
 importFrom(checkmate,test_subset)
 importFrom(checkmate,vname)

R/epiprocess.R

@@ -8,6 +8,7 @@
 #'             assert_logical assert_list assert_character assert_class
 #'             assert_int assert_numeric check_data_frame vname check_atomic
 #'             anyInfinite test_subset test_set_equal checkInt expect_class
+#'             testInt
 #' @importFrom cli cli_abort cli_warn
 #' @importFrom rlang %||%
 #' @name epiprocess

R/grouped_epi_archive.R

@@ -204,10 +204,16 @@ ungroup.grouped_epi_archive <- function(x, ...) {
 #' @importFrom rlang !! !!! enquo quo_is_missing enquos is_quosure sym syms
 #'  env missing_arg
 #' @export
-epix_slide.grouped_epi_archive <- function(x, f, ..., before, ref_time_values,
-                                           time_step, new_col_name = "slide_value",
-                                           as_list_col = FALSE, names_sep = "_",
-                                           all_versions = FALSE) {
+epix_slide.grouped_epi_archive <- function(
+    x,
+    f,
+    ...,
+    before = Inf,
+    ref_time_values = NULL,
+    new_col_name = "slide_value",
+    as_list_col = FALSE,
+    names_sep = "_",
+    all_versions = FALSE) {
   # Perform some deprecated argument checks without using `<param> =
   # deprecated()` in the function signature, because they are from
   # early development versions and much more likely to be clutter than
@@ -231,7 +237,7 @@ epix_slide.grouped_epi_archive <- function(x, f, ..., before, ref_time_values,
         ", class = "epiprocess__epix_slide_all_rows_parameter_deprecated")
   }
 
-  if (missing(ref_time_values)) {
+  if (is.null(ref_time_values)) {
     ref_time_values <- epix_slide_ref_time_values_default(x$private$ungrouped)
   } else {
     assert_numeric(ref_time_values, min.len = 1L, null.ok = FALSE, any.missing = FALSE)
@@ -247,19 +253,9 @@ epix_slide.grouped_epi_archive <- function(x, f, ..., before, ref_time_values,
   }
 
   # Validate and pre-process `before`:
-  if (missing(before)) {
-    cli_abort("`before` is required (and must be passed by name);
-                if you did not want to apply a sliding window but rather
-                to map `epix_as_of` and `f` across various `ref_time_values`,
-                pass a large `before` value (e.g., if time steps are days,
-                `before=365000`).")
+  if (!(anyInfinite(before) || testInt(before, lower = 0L, null.ok = FALSE, na.ok = FALSE))) {
+    cli_abort("`before` must be a non-negative integer or Inf.")
   }
-  before <- vctrs::vec_cast(before, integer())
-  assert_int(before, lower = 0L, null.ok = FALSE, na.ok = FALSE)
-
-  # If a custom time step is specified, then redefine units
-
-  if (!missing(time_step)) before <- time_step(before)
 
   # Symbolize column name
   new_col <- sym(new_col_name)

R/methods-epi_archive.R

@@ -790,9 +790,8 @@ epix_slide <- function(
     x,
     f,
     ...,
-    before,
-    ref_time_values,
-    time_step,
+    before = Inf,
+    ref_time_values = NULL,
     new_col_name = "slide_value",
     as_list_col = FALSE,
     names_sep = "_",
@@ -803,19 +802,24 @@ epix_slide <- function(
 
 #' @rdname epix_slide
 #' @export
-epix_slide.epi_archive <- function(x, f, ..., before, ref_time_values,
-                                   time_step, new_col_name = "slide_value",
-                                   as_list_col = FALSE, names_sep = "_",
-                                   all_versions = FALSE) {
+epix_slide.epi_archive <- function(
+    x,
+    f,
+    ...,
+    before = Inf,
+    ref_time_values = NULL,
+    new_col_name = "slide_value",
+    as_list_col = FALSE,
+    names_sep = "_",
+    all_versions = FALSE) {
   # For an "ungrouped" slide, treat all rows as belonging to one big
   # group (group by 0 vars), like `dplyr::summarize`, and let the
   # resulting `grouped_epi_archive` handle the slide:
   epix_slide(
     group_by(x),
     f,
     ...,
-    before = before, ref_time_values = ref_time_values,
-    time_step = time_step, new_col_name = new_col_name,
+    before = before, ref_time_values = ref_time_values, new_col_name = new_col_name,
     as_list_col = as_list_col, names_sep = names_sep,
     all_versions = all_versions
   ) %>%

tests/testthat/test-epix_slide.R

@@ -174,30 +174,18 @@ test_that("epix_slide works as intended with `as_list_col=TRUE`", {
 })
 
 test_that("epix_slide `before` validation works", {
-  expect_error(
-    xx %>% epix_slide(f = ~ sum(.x$binary)),
-    "`before` is required"
-  )
   expect_error(
     xx %>% epix_slide(f = ~ sum(.x$binary), before = NA),
-    "Assertion on 'before' failed: May not be NA"
+    "`before` must be a non-negative integer or Inf."
   )
   expect_error(
     xx %>% epix_slide(f = ~ sum(.x$binary), before = -1),
-    "Assertion on 'before' failed: Element 1 is not >= 0"
+    "`before` must be a non-negative integer or Inf."
   )
   expect_error(
     xx %>% epix_slide(f = ~ sum(.x$binary), before = 1.5),
-    regexp = "before",
-    class = "vctrs_error_incompatible_type"
-  )
-  # We might want to allow this at some point (issue #219):
-  expect_error(
-    xx %>% epix_slide(f = ~ sum(.x$binary), before = Inf),
-    regexp = "before",
-    class = "vctrs_error_incompatible_type"
+    "`before` must be a non-negative integer or Inf."
   )
-  expect_error(xx %>% epix_slide(f = ~ sum(.x$binary)), "`before` is required")
   # These `before` values should be accepted:
   expect_error(
     xx %>% epix_slide(f = ~ sum(.x$binary), before = 0),