Update epix_slide before docs, add validation

- Try to describe `before` in a similar manner as the PR for `epi_slide`
  `before`&`after` currently does.
- Fix some example discussion not yet adjusted for `before` being off by one
  relative to `n`.
- Update some discussion about differences between `epix_slide` and `epi_slide`
  `time_value` windows.
- Add tests for the `before` validation code

Author: lcbrooks

R/archive.R

@@ -597,7 +597,20 @@ epi_archive =
               ref_time_values = ref_time_values[ref_time_values %in%
                                                 unique(self$DT$time_value)]
             }
-              
+
+            # Validate and pre-process `before`:
+            if (missing(before)) {
+              Abort("`before` is required (and must be passed by name);
+                     if you did not want to apply a sliding window but rather
+                     to map `as_of` and `f` across various `ref_time_values`,
+                     pass a large `before` value (e.g., if time steps are days,
+                     `before=365000`).")
+            }
+            before <- vctrs::vec_cast(before, integer())
+            if (length(before) != 1L || is.na(before) || before < 0L) {
+              Abort("`before` must be length-1, non-NA, non-negative")
+            }
+
             # If a custom time step is specified, then redefine units 
             if (!missing(time_step)) before <- time_step(before)
 

R/methods-epi_archive.R

@@ -358,10 +358,21 @@ epix_merge = function(x, y,
 #' @param ... Additional arguments to pass to the function or formula specified
 #'   via `f`. Alternatively, if `f` is missing, then the current argument is
 #'   interpreted as an expression for tidy evaluation.
-#' @param before Number of time steps to use in the running window. For example,
-#'   if `before = 7`, and one time step is one day, then to produce a value on
-#'   January 7 we apply the given function or formula to data in between January
-#'   1 and 7.
+#' @param before How far `before` each `ref_time_value` should the sliding
+#'   window extend? If provided, should be a single, non-NA,
+#'   [integer-compatible][vctrs::vec_cast] number of time steps. This window
+#'   endpoint is inclusive. For example, if `before = 7`, and one time step is
+#'   one day, then to produce a value for a `ref_time_value` of January 8, we
+#'   apply the given function or formula to data (for each group present) with
+#'   `time_value`s from January 1 onward, as they were reported on January 8.
+#'   For typical disease surveillance sources, this will not include any data
+#'   with a `time_value` of January 8, and, depending on the amount of reporting
+#'   latency, may not include January 7 or even earlier `time_value`s. (If
+#'   instead the archive were to hold nowcasts instead of regular surveillance
+#'   data, then we would indeed expect data for `time_value` January 8. If it
+#'   were to hold forecasts, then we would expect data for `time_value`s after
+#'   January 8, and the sliding window would extend as far after each
+#'   `ref_time_value` as needed to include all such `time_value`s.)
 #' @param group_by The variable(s) to group by before slide computation. If
 #'   missing, then the keys in the underlying data table, excluding `time_value`
 #'   and `version`, will be used for grouping. To omit a grouping entirely, use
@@ -396,10 +407,14 @@ epix_merge = function(x, y,
 #'   values.
 #'
 #' @details Two key distinctions between inputs to the current function and
-#'   `epi_slide()`:
-#'   1. `epix_slide()` uses windows that are **always right-aligned** (in
-#'   `epi_slide()`, custom alignments could be specified using the `align` or
-#'   `before` arguments).
+#'   [`epi_slide()`]:
+#'   1. `epix_slide()` doesn't accept an `after` argument; its windows extend
+#'   from `before` time steps before a given `ref_time_value` through the last
+#'   `time_value` available as of version `ref_time_value` (typically, this
+#'   won't include `ref_time_value` itself, as observations about a particular
+#'   time interval (e.g., day) are only published after that time interval ends);
+#'   `epi_slide` windows extend from `before` time steps before a
+#'   `ref_time_value` through `after` time steps after `ref_time_value`.
 #'   2. `epix_slide()` uses a `group_by` to specify the grouping upfront (in
 #'   `epi_slide()`, this would be accomplished by a preceding function call to
 #'   `dplyr::group_by()`).

man/epix_slide.Rd

@@ -4,17 +4,17 @@ test_that("epix_slide only works on an epi_archive",{
   expect_error(epix_slide(data.frame(x=1)))
 })
 
+x <- tibble::tribble(~version, ~time_value, ~binary,
+                     4,       c(1:3),       2^(1:3),
+                     5,       c(1:2,4),     2^(4:6),
+                     6,       c(1:2,4:5),   2^(7:10),
+                     7,       2:6,          2^(11:15)) %>%
+  tidyr::unnest(c(time_value,binary))
+
+xx <- bind_cols(geo_value = rep("x",15), x) %>%
+  as_epi_archive()
+
 test_that("epix_slide works as intended",{
-  x <- tibble::tribble(~version, ~time_value, ~binary,
-                       4,       c(1:3),       2^(1:3),
-                       5,       c(1:2,4),     2^(4:6),
-                       6,       c(1:2,4:5),   2^(7:10),
-                       7,       2:6,          2^(11:15)) %>%
-    tidyr::unnest(c(time_value,binary))
-  
-  xx <- bind_cols(geo_value = rep("x",15), x) %>%
-    as_epi_archive()
-  
   xx1 <- epix_slide(x = xx,
                     f = ~ sum(.x$binary),
                     before = 2,
@@ -38,3 +38,28 @@ test_that("epix_slide works as intended",{
 
   expect_identical(xx1,xx3) # This and * Imply xx2 and xx3 are identical
 })
+
+test_that("epix_slide `before` validation works", {
+  expect_error(xx$slide(f = ~ sum(.x$binary)),
+               "`before` is required")
+  expect_error(xx$slide(f = ~ sum(.x$binary), before=NA),
+               "`before`.*NA")
+  expect_error(xx$slide(f = ~ sum(.x$binary), before=-1),
+               "`before`.*negative")
+  expect_error(xx$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$slide(f = ~ sum(.x$binary), before=Inf),
+               regexp="before",
+               class="vctrs_error_incompatible_type")
+  # (wrapper shouldn't introduce a value:)
+  expect_error(epix_slide(xx, f = ~ sum(.x$binary)), "`before` is required")
+  # These `before` values should be accepted:
+  expect_error(xx$slide(f = ~ sum(.x$binary), before=0),
+               NA)
+  expect_error(xx$slide(f = ~ sum(.x$binary), before=2L),
+               NA)
+  expect_error(xx$slide(f = ~ sum(.x$binary), before=365000),
+               NA)
+})