From c8015602b0b425b0a4a67ef5df02a34d8212a16a Mon Sep 17 00:00:00 2001 From: "Logan C. Brooks" Date: Mon, 30 Sep 2024 16:56:35 -0700 Subject: [PATCH 1/8] Fix validate_version_bound typo --- DESCRIPTION | 2 +- R/archive.R | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 790b36a5..784496b2 100755 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,7 +1,7 @@ Type: Package Package: epiprocess Title: Tools for basic signal processing in epidemiology -Version: 0.9.0 +Version: 0.10.0 Authors@R: c( person("Jacob", "Bien", role = "ctb"), person("Logan", "Brooks", , "lcbrooks@andrew.cmu.edu", role = c("aut", "cre")), diff --git a/R/archive.R b/R/archive.R index e877d397..d8102165 100644 --- a/R/archive.R +++ b/R/archive.R @@ -25,7 +25,7 @@ #' @noRd validate_version_bound <- function(version_bound, x, na_ok = FALSE, version_bound_arg = rlang::caller_arg(version_bound), - x_arg = rlang::caller_arg(version_bound)) { + x_arg = rlang::caller_arg(x)) { if (is.null(version_bound)) { cli_abort( "{version_bound_arg} cannot be NULL", From d9cf933e03bb186255f8b49f404b59ccfab2cf28 Mon Sep 17 00:00:00 2001 From: "Logan C. Brooks" Date: Tue, 1 Oct 2024 15:23:21 -0700 Subject: [PATCH 2/8] fix(validate_slide_window_arg): match doc'd Date x difftime reqs --- R/utils.R | 2 +- tests/testthat/test-utils.R | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/R/utils.R b/R/utils.R index bb30264a..88c7eaa0 100644 --- a/R/utils.R +++ b/R/utils.R @@ -1001,7 +1001,7 @@ validate_slide_window_arg <- function(arg, time_type, lower = 1, allow_inf = TRU msg <- "" if (!identical(arg, Inf)) { if (time_type == "day") { - if (!test_int(arg, lower = 0L) && !(inherits(arg, "difftime") && units(arg) == "days")) { + if (!test_int(arg, lower = 0L) || inherits(arg, "difftime") && units(arg) != "days") { msg <- glue::glue_collapse(c("difftime with units in days", "non-negative integer", "Inf"), " or ") } } else if (time_type == "week") { diff --git a/tests/testthat/test-utils.R b/tests/testthat/test-utils.R index a159f98e..5767751d 100644 --- a/tests/testthat/test-utils.R +++ b/tests/testthat/test-utils.R @@ -306,7 +306,8 @@ test_that("validate_slide_window_arg works", { } expect_no_error(validate_slide_window_arg(as.difftime(1, units = "days"), "day")) expect_no_error(validate_slide_window_arg(1, "day")) - expect_no_error(validate_slide_window_arg(as.difftime(1, units = "weeks"), "day")) + expect_error(validate_slide_window_arg(as.difftime(1, units = "weeks"), "day")) + expect_error(validate_slide_window_arg(as.difftime(1, units = "secs"), "day")) expect_no_error(validate_slide_window_arg(as.difftime(1, units = "weeks"), "week")) expect_error(validate_slide_window_arg(1, "week")) From 4f37e4aa245b7fa1f5fc182a4292ef43b88a0c12 Mon Sep 17 00:00:00 2001 From: "Logan C. Brooks" Date: Tue, 1 Oct 2024 16:06:47 -0700 Subject: [PATCH 3/8] Complete docs and match code&messages wrt .window_size validation * Mention what's acceptable for yearmonth time_type * Mention Inf in validation errors as acceptable iff it's actually acceptable. * Reject any other strange "int" classes that pass test_int. (It rejects Date and POSIXt, but perhaps there are others.) * Refactor to use helper function test_sensible_int instead of test_int (as latter accepts difftimes and makes logic look confusing). --- R/slide.R | 6 +-- R/utils.R | 83 +++++++++++++++++++++----------- man-roxygen/basic-slide-params.R | 3 +- man/epi_slide.Rd | 3 +- man/epi_slide_mean.Rd | 5 +- man/epi_slide_opt.Rd | 5 +- man/epi_slide_sum.Rd | 5 +- man/test_sensible_int.Rd | 28 +++++++++++ tests/testthat/test-utils.R | 24 ++++++--- 9 files changed, 118 insertions(+), 44 deletions(-) create mode 100644 man/test_sensible_int.Rd diff --git a/R/slide.R b/R/slide.R index 5a7fbd6a..b971b9d5 100644 --- a/R/slide.R +++ b/R/slide.R @@ -576,7 +576,7 @@ get_before_after_from_window <- function(window_size, align, time_type) { #' ungroup() epi_slide_opt <- function( .x, .col_names, .f, ..., - .window_size = 1, .align = c("right", "center", "left"), + .window_size = NULL, .align = c("right", "center", "left"), .ref_time_values = NULL, .all_rows = FALSE) { assert_class(.x, "epi_df") @@ -902,7 +902,7 @@ epi_slide_opt <- function( #' ungroup() epi_slide_mean <- function( .x, .col_names, ..., - .window_size = 1, .align = c("right", "center", "left"), + .window_size = NULL, .align = c("right", "center", "left"), .ref_time_values = NULL, .all_rows = FALSE) { # Deprecated argument handling provided_args <- rlang::call_args_names(rlang::call_match()) @@ -979,7 +979,7 @@ epi_slide_mean <- function( #' ungroup() epi_slide_sum <- function( .x, .col_names, ..., - .window_size = 1, .align = c("right", "center", "left"), + .window_size = NULL, .align = c("right", "center", "left"), .ref_time_values = NULL, .all_rows = FALSE) { # Deprecated argument handling provided_args <- rlang::call_args_names(rlang::call_match()) diff --git a/R/utils.R b/R/utils.R index 88c7eaa0..6b44081f 100644 --- a/R/utils.R +++ b/R/utils.R @@ -982,14 +982,31 @@ guess_period.POSIXt <- function(time_values, time_values_arg = rlang::caller_arg as.numeric(NextMethod(), units = "secs") } -validate_slide_window_arg <- function(arg, time_type, lower = 1, allow_inf = TRUE, arg_name = rlang::caller_arg(arg)) { - if (!checkmate::test_scalar(arg) || arg < lower) { - cli_abort( - "Slide function expected `{arg_name}` to be a non-null, scalar integer >= {lower}.", - class = "epiprocess__validate_slide_window_arg" +#' Is `x` an "int" with a sensible class? TRUE/FALSE +#' +#' Like [`checkmate::test_int`] but disallowing some non-sensible classes that +#' `test_int` accepts, such as `difftime`s. We rely on [`is.numeric`] to +#' determine class appropriateness; note that `is.numeric` is NOT simply +#' checking for the class to be "numeric" (or else we'd fail on integer class). +#' +#' @param x object +#' @return Boolean +#' +#' @importFrom checkmate test_int +#' @keywords internal +test_sensible_int <- function(x, na.ok = FALSE, lower = -Inf, upper = Inf, + tol = sqrt(.Machine$double.eps), null.ok = FALSE) { + if (null.ok && is.null(x)) { + TRUE + } else { + is.numeric(x) && test_int(x, + na.ok = FALSE, lower = -Inf, upper = Inf, + tol = sqrt(.Machine$double.eps) ) } +} +validate_slide_window_arg <- function(arg, time_type, lower = 1, allow_inf = TRUE, arg_name = rlang::caller_arg(arg)) { if (time_type == "custom") { cli_abort( "Unsure how to interpret slide units with a custom time type. Consider converting your time @@ -999,31 +1016,43 @@ validate_slide_window_arg <- function(arg, time_type, lower = 1, allow_inf = TRU } msg <- "" - if (!identical(arg, Inf)) { - if (time_type == "day") { - if (!test_int(arg, lower = 0L) || inherits(arg, "difftime") && units(arg) != "days") { - msg <- glue::glue_collapse(c("difftime with units in days", "non-negative integer", "Inf"), " or ") - } - } else if (time_type == "week") { - if (!(inherits(arg, "difftime") && units(arg) == "weeks")) { - msg <- glue::glue_collapse(c("difftime with units in weeks", "Inf"), " or ") - } - } else if (time_type == "yearmonth") { - if (!test_int(arg, lower = 0L) || inherits(arg, "difftime")) { - msg <- glue::glue_collapse(c("non-negative integer", "Inf"), " or ") - } - } else if (time_type == "integer") { - if (!test_int(arg, lower = 0L) || inherits(arg, "difftime")) { - msg <- glue::glue_collapse(c("non-negative integer", "Inf"), " or ") - } - } else { - msg <- glue::glue_collapse(c("difftime", "non-negative integer", "Inf"), " or ") - } + inf_if_okay <- if (allow_inf) { + "Inf" } else { - if (!allow_inf) { - msg <- glue::glue_collapse(c("a difftime", "a non-negative integer"), " or ") + character(0L) + } + + if (time_type == "day") { + if (!(test_sensible_int(arg, lower = 0L) || + inherits(arg, "difftime") && length(arg) == 1L && units(arg) == "days" || + allow_inf && identical(arg, Inf) + )) { + msg <- glue::glue_collapse(c("length-1 difftime with units in days", "non-negative integer", inf_if_okay), " or ") } + } else if (time_type == "week") { + if (!(inherits(arg, "difftime") && length(arg) == 1L && units(arg) == "weeks" || + allow_inf && identical(arg, Inf) + )) { + msg <- glue::glue_collapse(c("length-1 difftime with units in weeks", inf_if_okay), " or ") + } + } else if (time_type == "yearmonth") { + if (!(test_sensible_int(arg, lower = 0L) || + allow_inf && identical(arg, Inf) + )) { + msg <- glue::glue_collapse(c("non-negative integer", inf_if_okay), " or ") + } + } else if (time_type == "integer") { + if (!(test_sensible_int(arg, lower = 0L) || + allow_inf && identical(arg, Inf) + )) { + msg <- glue::glue_collapse(c("non-negative integer", inf_if_okay), " or ") + } + } else { + cli_abort('`epiprocess` internal error: unrecognized time_type: "{time_type}"', + class = "epiprocess__unrecognized_time_type" + ) } + if (msg != "") { cli_abort( "Slide function expected `{arg_name}` to be a {msg}.", diff --git a/man-roxygen/basic-slide-params.R b/man-roxygen/basic-slide-params.R index 8a63a817..dfa2512f 100644 --- a/man-roxygen/basic-slide-params.R +++ b/man-roxygen/basic-slide-params.R @@ -10,7 +10,8 @@ #' with units "days" #' - if time_type is Date and the cadence is weekly, then `.window_size` must #' be a difftime with units "weeks" -#' - if time_type is an integer, then `.window_size` must be an integer +#' - if time_type is an yearmonth or integer, then `.window_size` must be an +#' integer #' #' @param .align The alignment of the sliding window. If `right` (default), then #' the window has its end at the reference time; if `center`, then the window is diff --git a/man/epi_slide.Rd b/man/epi_slide.Rd index 74929eb1..8029e2a4 100644 --- a/man/epi_slide.Rd +++ b/man/epi_slide.Rd @@ -57,7 +57,8 @@ an integer (which will be interpreted in units of days) or a difftime with units "days" \item if time_type is Date and the cadence is weekly, then \code{.window_size} must be a difftime with units "weeks" -\item if time_type is an integer, then \code{.window_size} must be an integer +\item if time_type is an yearmonth or integer, then \code{.window_size} must be an +integer }} \item{.align}{The alignment of the sliding window. If \code{right} (default), then diff --git a/man/epi_slide_mean.Rd b/man/epi_slide_mean.Rd index 09faefb6..75b83b10 100644 --- a/man/epi_slide_mean.Rd +++ b/man/epi_slide_mean.Rd @@ -8,7 +8,7 @@ epi_slide_mean( .x, .col_names, ..., - .window_size = 1, + .window_size = NULL, .align = c("right", "center", "left"), .ref_time_values = NULL, .all_rows = FALSE @@ -44,7 +44,8 @@ an integer (which will be interpreted in units of days) or a difftime with units "days" \item if time_type is Date and the cadence is weekly, then \code{.window_size} must be a difftime with units "weeks" -\item if time_type is an integer, then \code{.window_size} must be an integer +\item if time_type is an yearmonth or integer, then \code{.window_size} must be an +integer }} \item{.align}{The alignment of the sliding window. If \code{right} (default), then diff --git a/man/epi_slide_opt.Rd b/man/epi_slide_opt.Rd index dcaab3f8..24b813f0 100644 --- a/man/epi_slide_opt.Rd +++ b/man/epi_slide_opt.Rd @@ -10,7 +10,7 @@ epi_slide_opt( .col_names, .f, ..., - .window_size = 1, + .window_size = NULL, .align = c("right", "center", "left"), .ref_time_values = NULL, .all_rows = FALSE @@ -59,7 +59,8 @@ an integer (which will be interpreted in units of days) or a difftime with units "days" \item if time_type is Date and the cadence is weekly, then \code{.window_size} must be a difftime with units "weeks" -\item if time_type is an integer, then \code{.window_size} must be an integer +\item if time_type is an yearmonth or integer, then \code{.window_size} must be an +integer }} \item{.align}{The alignment of the sliding window. If \code{right} (default), then diff --git a/man/epi_slide_sum.Rd b/man/epi_slide_sum.Rd index 0c83c432..2cf05cca 100644 --- a/man/epi_slide_sum.Rd +++ b/man/epi_slide_sum.Rd @@ -8,7 +8,7 @@ epi_slide_sum( .x, .col_names, ..., - .window_size = 1, + .window_size = NULL, .align = c("right", "center", "left"), .ref_time_values = NULL, .all_rows = FALSE @@ -44,7 +44,8 @@ an integer (which will be interpreted in units of days) or a difftime with units "days" \item if time_type is Date and the cadence is weekly, then \code{.window_size} must be a difftime with units "weeks" -\item if time_type is an integer, then \code{.window_size} must be an integer +\item if time_type is an yearmonth or integer, then \code{.window_size} must be an +integer }} \item{.align}{The alignment of the sliding window. If \code{right} (default), then diff --git a/man/test_sensible_int.Rd b/man/test_sensible_int.Rd new file mode 100644 index 00000000..eda7aacb --- /dev/null +++ b/man/test_sensible_int.Rd @@ -0,0 +1,28 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/utils.R +\name{test_sensible_int} +\alias{test_sensible_int} +\title{Is \code{x} an "int" with a sensible class? TRUE/FALSE} +\usage{ +test_sensible_int( + x, + na.ok = FALSE, + lower = -Inf, + upper = Inf, + tol = sqrt(.Machine$double.eps), + null.ok = FALSE +) +} +\arguments{ +\item{x}{object} +} +\value{ +Boolean +} +\description{ +Like \code{\link[checkmate:checkInt]{checkmate::test_int}} but disallowing some non-sensible classes that +\code{test_int} accepts, such as \code{difftime}s. We rely on \code{\link{is.numeric}} to +determine class appropriateness; note that \code{is.numeric} is NOT simply +checking for the class to be "numeric" (or else we'd fail on integer class). +} +\keyword{internal} diff --git a/tests/testthat/test-utils.R b/tests/testthat/test-utils.R index 5767751d..8b942f28 100644 --- a/tests/testthat/test-utils.R +++ b/tests/testthat/test-utils.R @@ -306,16 +306,28 @@ test_that("validate_slide_window_arg works", { } expect_no_error(validate_slide_window_arg(as.difftime(1, units = "days"), "day")) expect_no_error(validate_slide_window_arg(1, "day")) - expect_error(validate_slide_window_arg(as.difftime(1, units = "weeks"), "day")) - expect_error(validate_slide_window_arg(as.difftime(1, units = "secs"), "day")) + expect_error(validate_slide_window_arg(as.difftime(1, units = "weeks"), "day"), + class = "epiprocess__validate_slide_window_arg" + ) + expect_error(validate_slide_window_arg(as.difftime(1, units = "secs"), "day"), + class = "epiprocess__validate_slide_window_arg" + ) expect_no_error(validate_slide_window_arg(as.difftime(1, units = "weeks"), "week")) - expect_error(validate_slide_window_arg(1, "week")) + expect_error(validate_slide_window_arg(1, "week"), + class = "epiprocess__validate_slide_window_arg" + ) expect_no_error(validate_slide_window_arg(1, "integer")) - expect_error(validate_slide_window_arg(as.difftime(1, units = "days"), "integer")) - expect_error(validate_slide_window_arg(as.difftime(1, units = "weeks"), "integer")) + expect_error(validate_slide_window_arg(as.difftime(1, units = "days"), "integer"), + class = "epiprocess__validate_slide_window_arg" + ) + expect_error(validate_slide_window_arg(as.difftime(1, units = "weeks"), "integer"), + class = "epiprocess__validate_slide_window_arg" + ) expect_no_error(validate_slide_window_arg(1, "yearmonth")) - expect_error(validate_slide_window_arg(as.difftime(1, units = "weeks"), "yearmonth")) + expect_error(validate_slide_window_arg(as.difftime(1, units = "weeks"), "yearmonth"), + class = "epiprocess__validate_slide_window_arg" + ) }) From 34ad569a2fb43648926ec7f8adbb8da2f2003e73 Mon Sep 17 00:00:00 2001 From: "Logan C. Brooks" Date: Tue, 1 Oct 2024 16:35:16 -0700 Subject: [PATCH 4/8] Pacify lintr --- R/utils.R | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/R/utils.R b/R/utils.R index 6b44081f..8a03c626 100644 --- a/R/utils.R +++ b/R/utils.R @@ -994,8 +994,8 @@ guess_period.POSIXt <- function(time_values, time_values_arg = rlang::caller_arg #' #' @importFrom checkmate test_int #' @keywords internal -test_sensible_int <- function(x, na.ok = FALSE, lower = -Inf, upper = Inf, - tol = sqrt(.Machine$double.eps), null.ok = FALSE) { +test_sensible_int <- function(x, na.ok = FALSE, lower = -Inf, upper = Inf, # nolint: object_name_linter + tol = sqrt(.Machine$double.eps), null.ok = FALSE) { # nolint: object_name_linter if (null.ok && is.null(x)) { TRUE } else { @@ -1022,6 +1022,7 @@ validate_slide_window_arg <- function(arg, time_type, lower = 1, allow_inf = TRU character(0L) } + # nolint start: indentation_linter. if (time_type == "day") { if (!(test_sensible_int(arg, lower = 0L) || inherits(arg, "difftime") && length(arg) == 1L && units(arg) == "days" || @@ -1052,6 +1053,7 @@ validate_slide_window_arg <- function(arg, time_type, lower = 1, allow_inf = TRU class = "epiprocess__unrecognized_time_type" ) } + # nolint end if (msg != "") { cli_abort( From 44d354d4851c7962475eadd95d94657d68b3b823 Mon Sep 17 00:00:00 2001 From: Dmitry Shemetov Date: Tue, 1 Oct 2024 20:54:53 -0700 Subject: [PATCH 5/8] fix: epi_slide_opt window_size validation, fix test, fix bug --- R/slide.R | 54 ++++++++------------------------- R/utils.R | 11 +++---- tests/testthat/test-epi_slide.R | 12 ++------ 3 files changed, 18 insertions(+), 59 deletions(-) diff --git a/R/slide.R b/R/slide.R index b971b9d5..b862693a 100644 --- a/R/slide.R +++ b/R/slide.R @@ -678,46 +678,16 @@ epi_slide_opt <- function( ref_time_values <- sort(.ref_time_values) # Handle window arguments - align <- rlang::arg_match(.align) + .align <- rlang::arg_match(.align) time_type <- attr(.x, "metadata")$time_type - validate_slide_window_arg(.window_size, time_type) - if (identical(.window_size, Inf)) { - if (align == "right") { - before <- Inf - if (time_type %in% c("day", "week")) { - after <- as.difftime(0, units = glue::glue("{time_type}s")) - } else { - after <- 0 - } - } else { - cli_abort( - "`epi_slide`: center and left alignment are not supported with an infinite window size." - ) - } - } else { - if (align == "right") { - before <- .window_size - 1 - if (time_type %in% c("day", "week")) { - after <- as.difftime(0, units = glue::glue("{time_type}s")) - } else { - after <- 0 - } - } else if (align == "center") { - # For .window_size = 5, before = 2, after = 2. For .window_size = 4, before = 2, after = 1. - before <- floor(.window_size / 2) - after <- .window_size - before - 1 - } else if (align == "left") { - if (time_type %in% c("day", "week")) { - before <- as.difftime(0, units = glue::glue("{time_type}s")) - } else { - before <- 0 - } - after <- .window_size - 1 - } + if (is.null(.window_size)) { + cli_abort("epi_slide: `.window_size` must be specified.") } + validate_slide_window_arg(.window_size, time_type) + window_args <- get_before_after_from_window(.window_size, .align, time_type) # Make a complete date sequence between min(.x$time_value) and max(.x$time_value). - date_seq_list <- full_date_seq(.x, before, after, time_type) + date_seq_list <- full_date_seq(.x, window_args$before, window_args$after, time_type) all_dates <- date_seq_list$all_dates pad_early_dates <- date_seq_list$pad_early_dates pad_late_dates <- date_seq_list$pad_late_dates @@ -786,16 +756,16 @@ epi_slide_opt <- function( # `before` and `after` params. Right-aligned `frollmean` results' # `ref_time_value`s will be `after` timesteps ahead of where they should # be; shift results to the left by `after` timesteps. - if (before != Inf) { - window_size <- before + after + 1L + if (window_args$before != Inf) { + window_size <- window_args$before + window_args$after + 1L roll_output <- .f(x = .data_group[, col_names_chr], n = window_size, ...) } else { window_size <- list(seq_along(.data_group$time_value)) roll_output <- .f(x = .data_group[, col_names_chr], n = window_size, adaptive = TRUE, ...) } - if (after >= 1) { + if (window_args$after >= 1) { .data_group[, result_col_names] <- purrr::map(roll_output, function(.x) { - c(.x[(after + 1L):length(.x)], rep(NA, after)) + c(.x[(window_args$after + 1L):length(.x)], rep(NA, window_args$after)) }) } else { .data_group[, result_col_names] <- roll_output @@ -805,8 +775,8 @@ epi_slide_opt <- function( for (i in seq_along(col_names_chr)) { .data_group[, result_col_names[i]] <- .f( x = .data_group[[col_names_chr[i]]], - before = as.numeric(before), - after = as.numeric(after), + before = as.numeric(window_args$before), + after = as.numeric(window_args$after), ... ) } diff --git a/R/utils.R b/R/utils.R index 8a03c626..29e60054 100644 --- a/R/utils.R +++ b/R/utils.R @@ -999,10 +999,7 @@ test_sensible_int <- function(x, na.ok = FALSE, lower = -Inf, upper = Inf, # nol if (null.ok && is.null(x)) { TRUE } else { - is.numeric(x) && test_int(x, - na.ok = FALSE, lower = -Inf, upper = Inf, - tol = sqrt(.Machine$double.eps) - ) + is.numeric(x) && test_int(x, na.ok = na.ok, lower = lower, upper = upper, tol = tol) } } @@ -1024,7 +1021,7 @@ validate_slide_window_arg <- function(arg, time_type, lower = 1, allow_inf = TRU # nolint start: indentation_linter. if (time_type == "day") { - if (!(test_sensible_int(arg, lower = 0L) || + if (!(test_sensible_int(arg, lower = lower) || inherits(arg, "difftime") && length(arg) == 1L && units(arg) == "days" || allow_inf && identical(arg, Inf) )) { @@ -1037,13 +1034,13 @@ validate_slide_window_arg <- function(arg, time_type, lower = 1, allow_inf = TRU msg <- glue::glue_collapse(c("length-1 difftime with units in weeks", inf_if_okay), " or ") } } else if (time_type == "yearmonth") { - if (!(test_sensible_int(arg, lower = 0L) || + if (!(test_sensible_int(arg, lower = lower) || allow_inf && identical(arg, Inf) )) { msg <- glue::glue_collapse(c("non-negative integer", inf_if_okay), " or ") } } else if (time_type == "integer") { - if (!(test_sensible_int(arg, lower = 0L) || + if (!(test_sensible_int(arg, lower = lower) || allow_inf && identical(arg, Inf) )) { msg <- glue::glue_collapse(c("non-negative integer", inf_if_okay), " or ") diff --git a/tests/testthat/test-epi_slide.R b/tests/testthat/test-epi_slide.R index d644e9a7..f658bcf4 100644 --- a/tests/testthat/test-epi_slide.R +++ b/tests/testthat/test-epi_slide.R @@ -711,17 +711,9 @@ test_that("epi_slide_opt helper `full_date_seq` returns expected date values", { test_that("`epi_slide_opt` errors when passed non-`data.table`, non-`slider` functions", { reexport_frollmean <- data.table::frollmean - expect_no_error( - epi_slide_opt( - test_data, - .col_names = value, .f = reexport_frollmean - ) - ) + expect_no_error(epi_slide_opt(test_data, .col_names = value, .f = reexport_frollmean, .window_size = 7)) expect_error( - epi_slide_opt( - test_data, - .col_names = value, .f = mean - ), + epi_slide_opt(test_data, .col_names = value, .f = mean), class = "epiprocess__epi_slide_opt__unsupported_slide_function" ) }) From 28f5327120c1767403d6dddf636c5c884075ad23 Mon Sep 17 00:00:00 2001 From: "Logan C. Brooks" Date: Wed, 2 Oct 2024 11:21:01 -0700 Subject: [PATCH 6/8] "epi_slide:" -> "epi_slide_opt:" in a error for latter --- R/slide.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/slide.R b/R/slide.R index b862693a..5794b0ee 100644 --- a/R/slide.R +++ b/R/slide.R @@ -681,7 +681,7 @@ epi_slide_opt <- function( .align <- rlang::arg_match(.align) time_type <- attr(.x, "metadata")$time_type if (is.null(.window_size)) { - cli_abort("epi_slide: `.window_size` must be specified.") + cli_abort("epi_slide_opt: `.window_size` must be specified.") } validate_slide_window_arg(.window_size, time_type) window_args <- get_before_after_from_window(.window_size, .align, time_type) From c8e4d697b1d8fa43268e19f9f507f08e8c0e971c Mon Sep 17 00:00:00 2001 From: brookslogan Date: Wed, 2 Oct 2024 11:59:27 -0700 Subject: [PATCH 7/8] Fix version number in DESCRIPTION Co-authored-by: Dmitry Shemetov --- DESCRIPTION | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index 784496b2..f880d825 100755 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,7 +1,7 @@ Type: Package Package: epiprocess Title: Tools for basic signal processing in epidemiology -Version: 0.10.0 +Version: 0.9.1 Authors@R: c( person("Jacob", "Bien", role = "ctb"), person("Logan", "Brooks", , "lcbrooks@andrew.cmu.edu", role = c("aut", "cre")), From 6361878f021ea529b8874ed177a15fb4f671639c Mon Sep 17 00:00:00 2001 From: "Logan C. Brooks" Date: Wed, 2 Oct 2024 12:09:37 -0700 Subject: [PATCH 8/8] Update DESCRIPTION, NEWS.md --- DESCRIPTION | 2 +- NEWS.md | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index 790b36a5..8b80431c 100755 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,7 +1,7 @@ Type: Package Package: epiprocess Title: Tools for basic signal processing in epidemiology -Version: 0.9.0 +Version: 0.9.2 Authors@R: c( person("Jacob", "Bien", role = "ctb"), person("Logan", "Brooks", , "lcbrooks@andrew.cmu.edu", role = c("aut", "cre")), diff --git a/NEWS.md b/NEWS.md index ee04b7f3..18c146ce 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,6 +2,18 @@ Pre-1.0.0 numbering scheme: 0.x will indicate releases, while 0.x.y will indicate PR's. +# epiprocess 0.10 + +## Breaking changes + +- Removed `.window_size = 1` default from `epi_slide_{mean,sum,opt}`; this + argument is now mandatory, and should nearly always be greater than 1 except + for testing purposes. + +## Improvements + +- Improved validation of `.window_size` arguments. + # epiprocess 0.9 ## Breaking changes