Merge pull request #500 from cmu-delphi/optAllOfFix

adding explicit columns for `epi_slide_opt`'s multi column case

Author: dsweber2

DESCRIPTION

@@ -1,7 +1,7 @@
 Type: Package
 Package: epiprocess
 Title: Tools for basic signal processing in epidemiology
-Version: 0.8.3
+Version: 0.8.4
 Authors@R: c(
     person("Jacob", "Bien", role = "ctb"),
     person("Logan", "Brooks", email = "[email protected]", role = c("aut", "cre")),

NEWS.md

@@ -13,6 +13,7 @@ Pre-1.0.0 numbering scheme: 0.x will indicate releases, while 0.x.y will indicat
 ## Bug fixes
 
 - Fix `epi_slide_opt` (and related functions) to correctly handle `before=Inf`.
+  Also allow multiple columns specified as a list of strings.
 - Disallow `after=Inf` in slide functions, since it doesn't seem like a likely
   use case and complicates code.
 

R/slide.R

@@ -39,7 +39,7 @@
 #'
 #' @importFrom lubridate days weeks
 #' @importFrom dplyr bind_rows group_vars filter select
-#' @importFrom rlang .data .env !! enquo enquos sym env missing_arg
+#' @importFrom rlang .data .env !! enquos sym env missing_arg
 #' @export
 #' @seealso [`epi_slide_opt`] [`epi_slide_mean`] [`epi_slide_sum`]
 #' @examples
@@ -327,7 +327,7 @@ epi_slide <- function(x, f, ..., before = NULL, after = NULL, ref_time_values =
 #'
 #' @template opt-slide-details
 #'
-#' @importFrom dplyr bind_rows mutate %>% arrange tibble select
+#' @importFrom dplyr bind_rows mutate %>% arrange tibble select all_of
 #' @importFrom rlang enquo quo_get_expr as_label expr_label caller_arg
 #' @importFrom tidyselect eval_select
 #' @importFrom purrr map map_lgl
@@ -509,7 +509,11 @@ epi_slide_opt <- function(x, col_names, f, ..., before = NULL, after = NULL, ref
   # positions of user-provided `col_names` into string column names. We avoid
   # using `names(pos)` directly for robustness and in case we later want to
   # allow users to rename fields via tidyselection.
-  pos <- eval_select(rlang::enquo(col_names), data = x, allow_rename = FALSE)
+  if (class(quo_get_expr(enquo(col_names))) == "character") {
+    pos <- eval_select(all_of(col_names), data = x, allow_rename = FALSE)
+  } else {
+    pos <- eval_select(enquo(col_names), data = x, allow_rename = FALSE)
+  }
   col_names_chr <- names(x)[pos]
   # Always rename results to "slide_value_<original column name>".
   result_col_names <- paste0("slide_value_", col_names_chr)

man-roxygen/opt-slide-params.R

@@ -1,9 +1,9 @@
 #' @param col_names <[`tidy-select`][dplyr_tidy_select]> An unquoted column
-#'  name(e.g., `cases`), multiple column names (e.g., `c(cases, deaths)`), or
-#'  [other tidy-select expression][tidyselect::language]. Variable names can
-#'  be used as if they were positions in the data frame, so expressions like
-#'  `x:y` can be used to select a range of variables. If you have the desired
-#'  column names stored in a vector `vars`, use `col_names = all_of(vars)`.
+#'   name(e.g., `cases`), multiple column names (e.g., `c(cases, deaths)`),
+#'   [other tidy-select expression][tidyselect::language], or a vector of
+#'   characters (e.g. `c("cases", "deaths")`). Variable names can be used as if
+#'   they were positions in the data frame, so expressions like `x:y` can be
+#'   used to select a range of variables.
 #'
 #'   The tidy-selection renaming interface is not supported, and cannot be used
 #'   to provide output column names; if you want to customize the output column

man/epi_slide_mean.Rd

@@ -1274,6 +1274,31 @@ test_that("`epi_slide_opt` errors when passed non-`data.table`, non-`slider` fun
   )
 })
 
+multi_columns <- dplyr::bind_rows(
+  dplyr::tibble(geo_value = "ak", time_value = test_date + 1:200, value = 1:200, value2 = -1:-200),
+  dplyr::tibble(geo_value = "al", time_value = test_date + 1:5, value = -(1:5), value2 = 1:5)
+) %>%
+  as_epi_df() %>%
+  group_by(geo_value)
+
+test_that("no dplyr warnings from selecting multiple columns", {
+  expect_no_warning(
+    multi_slid <- epi_slide_mean(multi_columns, col_names = c("value", "value2"), before = 3L)
+  )
+  expect_equal(
+    names(multi_slid),
+    c("geo_value", "time_value", "value", "value2", "slide_value_value", "slide_value_value2")
+  )
+  expect_no_warning(
+    multi_slid_select <- epi_slide_mean(multi_columns, c(value, value2), before = 3L)
+  )
+  expect_equal(multi_slid_select, multi_slid)
+  expect_no_warning(
+    multi_slid_select <- epi_slide_mean(multi_columns, starts_with("value"), before = 3L)
+  )
+  expect_equal(multi_slid_select, multi_slid)
+})
+
 test_that("Inf works in before/after in slide and slide_opt", {
   # Daily data
   df <- dplyr::bind_rows(

man/epi_slide_opt.Rd

@@ -1,4 +1,4 @@
-library(dplyr)
+suppressPackageStartupMessages(library(dplyr))
 
 test_date <- as.Date("2020-01-01")
 

man/epi_slide_sum.Rd

@@ -1,6 +1,6 @@
 test_that("Grouping, regrouping, and ungrouping archives works as intended", {
   # From an example:
-  library(dplyr)
+  suppressPackageStartupMessages(library(dplyr))
   toy_archive <-
     tribble(
       ~geo_value, ~age_group, ~time_value, ~version, ~value,