Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adjust epi_slide_opt output naming; adjust epi_slide* autogrouping; fix tidyselect issue #564

Merged
merged 28 commits into from
Dec 9, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
ac12ec1
refactor(epi_slide_opt): simplify tidyselect
brookslogan Nov 6, 2024
c75de38
Fix typo + adjust naming of sliding median example
brookslogan Nov 7, 2024
fab1c08
feat(epi_slide_opt)!: add `.prefix =, .suffix =, .new_col_names =`
brookslogan Nov 6, 2024
79ed2f3
Use different automatic names for slides on logical columns
brookslogan Nov 12, 2024
05a0ca8
Update epi_slide_{sum,mean} examples w/ naming options, cleanup
brookslogan Nov 14, 2024
5ffaf2f
Tweak&fix epi_slide docs regarding packing, nesting
brookslogan Nov 14, 2024
e099fd0
Make epi_slide* all use autogrouping + make autogrouping temporary
brookslogan Nov 14, 2024
6d8bcf6
Update NEWS.md with slide naming & autogrouping changes, bugfix
brookslogan Nov 14, 2024
03155dd
docs(time_delta_to_n_steps): copyediting
brookslogan Nov 14, 2024
cac02c2
docs(time_delta_to_n_steps): fix inaccurate equivalency statement
brookslogan Nov 14, 2024
9cf5229
Remove invalid link to undocumented helper function
brookslogan Nov 14, 2024
632c8d9
Address CHECK lints
brookslogan Nov 15, 2024
ad15b8f
Add unit_time_delta to make time_delta_to_n_steps make sense
brookslogan Nov 15, 2024
2752903
Don't export unit_time_delta (yet)
brookslogan Nov 15, 2024
ed9524e
Address CHECKS+workflow: don't document epidatasets re-exports
brookslogan Nov 22, 2024
5cc0d15
Fix error class name that referred to different function
brookslogan Nov 22, 2024
9d64bda
Fix partial rename: aggregate_epi_df -> sum_groups_epi_df in messages
brookslogan Nov 22, 2024
5c68195
Describe output grouping in basic-slide-params.R
brookslogan Nov 22, 2024
d369527
docs: roughly describe grouping of epix_slide output
brookslogan Nov 22, 2024
6191b61
Rework `epi_slide_{opt,mean,sum}` examples as they are 1 help topic
brookslogan Nov 23, 2024
6ab73d5
feat(epi_slide_opt): guard against multiple .f matches
brookslogan Nov 23, 2024
c78bcca
Improve slide output packing example comments
brookslogan Nov 23, 2024
53505d3
docs: document (GHA)
brookslogan Nov 23, 2024
0352d7b
Make `as_epi_df` remove grouping
brookslogan Nov 26, 2024
081bd22
Bring back slide unique-key checks, make as_epi_df ukey checks faster
brookslogan Nov 26, 2024
79093a8
Fix _pkgdown.yml (data set topics removed; link epidatasets topics)
brookslogan Nov 26, 2024
b4bacf8
Mollify CHECK regarding epidatasets re-exports
brookslogan Nov 26, 2024
7bf4c6a
Add missing dplyr import in slide example
brookslogan Dec 9, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions NAMESPACE
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ importFrom(checkmate,assert_list)
importFrom(checkmate,assert_logical)
importFrom(checkmate,assert_numeric)
importFrom(checkmate,assert_scalar)
importFrom(checkmate,assert_string)
importFrom(checkmate,checkInt)
importFrom(checkmate,check_atomic)
importFrom(checkmate,check_data_frame)
Expand Down Expand Up @@ -176,6 +177,7 @@ importFrom(dplyr,summarize)
importFrom(dplyr,tibble)
importFrom(dplyr,ungroup)
importFrom(ggplot2,autoplot)
importFrom(glue,glue)
importFrom(lifecycle,deprecated)
importFrom(lubridate,as.period)
importFrom(lubridate,days)
Expand All @@ -189,7 +191,6 @@ importFrom(rlang,"%||%")
importFrom(rlang,.data)
importFrom(rlang,.env)
importFrom(rlang,arg_match)
importFrom(rlang,as_label)
importFrom(rlang,caller_arg)
importFrom(rlang,caller_env)
importFrom(rlang,check_dots_empty)
Expand All @@ -199,6 +200,7 @@ importFrom(rlang,env)
importFrom(rlang,expr_label)
importFrom(rlang,f_env)
importFrom(rlang,f_rhs)
importFrom(rlang,is_bare_integerish)
importFrom(rlang,is_environment)
importFrom(rlang,is_formula)
importFrom(rlang,is_function)
Expand All @@ -207,7 +209,7 @@ importFrom(rlang,is_quosure)
importFrom(rlang,list2)
importFrom(rlang,missing_arg)
importFrom(rlang,new_function)
importFrom(rlang,quo_get_expr)
importFrom(rlang,quo_get_env)
importFrom(rlang,quo_is_missing)
importFrom(rlang,sym)
importFrom(rlang,syms)
Expand All @@ -232,3 +234,4 @@ importFrom(tidyselect,starts_with)
importFrom(tsibble,as_tsibble)
importFrom(utils,capture.output)
importFrom(utils,tail)
importFrom(vctrs,vec_data)
22 changes: 16 additions & 6 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,10 @@ Pre-1.0.0 numbering scheme: 0.x will indicate releases, while 0.x.y will indicat
with `covid`. The data set previously named `jhu_confirmed_cumulative_num` has
been removed from the package, but a renamed version is has been removed from
the package, but a renamed version is still available in `epidatasets`.

## Bug fixes

- 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.
- `epi_slide_{sum,mean,opt}` have improved default output column names, and
additional arguments for specifying names: `.prefix`, `.suffix`,
`.new_col_names`. To obtain the old naming behavior, use `.prefix =
"slide_value_"`.

## Improvements

Expand All @@ -29,6 +27,18 @@ Pre-1.0.0 numbering scheme: 0.x will indicate releases, while 0.x.y will indicat
- Improved validation of `.window_size` arguments.
- Rewrote a lot of the package documentation to be more consistent and
informative. Simplified and streamlined the vignettes.
- `epi_slide_{sum,mean,opt}` on ungrouped `epi_df`s will now temporarily group
by `geo_value` and any `other_keys` for the slide operation rather than raise
an error about duplicated time values. `epi_slide`'s analogous automatic
grouping has been made temporary in order to match.

## Bug fixes

- 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.
- Fixed `epi_slide_{sum,mean,opt}` raising an error on certain tidyselect
expressions.

## Cleanup

Expand Down
11 changes: 5 additions & 6 deletions R/epi_df.R
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,6 @@ as_epi_df.tbl_df <- function(
as_of,
other_keys = character(),
...) {
# possible standard substitutions for time_value
x <- rename(x, ...)
x <- guess_column_name(x, "time_value", time_column_names())
x <- guess_column_name(x, "geo_value", geo_column_names())
Expand Down Expand Up @@ -282,11 +281,11 @@ as_epi_df.tbl_df <- function(
cli_abort("as_epi_df: `other_keys` can't include \".time_value_counts\"")
}

duplicated_time_values <- x %>%
group_by(across(all_of(c("geo_value", "time_value", other_keys)))) %>%
filter(dplyr::n() > 1) %>%
ungroup()
if (nrow(duplicated_time_values) > 0) {
if (anyDuplicated(x[c("geo_value", "time_value", other_keys)])) {
duplicated_time_values <- x %>%
group_by(across(all_of(c("geo_value", "time_value", other_keys)))) %>%
filter(dplyr::n() > 1) %>%
ungroup()
bad_data <- capture.output(duplicated_time_values)
cli_abort(
"as_epi_df: some groups in the data have duplicated time values. epi_df requires a unique time_value per group.",
Expand Down
5 changes: 4 additions & 1 deletion R/epiprocess-package.R
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#' @importFrom checkmate anyInfinite anyMissing assert assert_character
#' @importFrom checkmate assert_class assert_data_frame assert_int assert_list
#' @importFrom checkmate assert_logical assert_numeric assert_scalar checkInt
#' @importFrom checkmate assert_string
#' @importFrom checkmate check_atomic check_data_frame expect_class test_int
#' @importFrom checkmate check_names
#' @importFrom checkmate test_subset test_set_equal vname
Expand All @@ -16,6 +17,8 @@
#' @importFrom dplyr select
#' @importFrom lifecycle deprecated
#' @importFrom rlang %||%
#' @importFrom rlang is_bare_integerish
#' @importFrom vctrs vec_data
## usethis namespace: end
NULL

Expand All @@ -24,5 +27,5 @@ utils::globalVariables(c(
"fitted", ".response", "geo_value", "time_value",
"value", ".real", "lag", "max_value", "min_value",
"median_value", "spread", "rel_spread", "time_to",
"time_near_latest", "n_revisions"
"time_near_latest", "n_revisions", "min_lag", "max_lag"
))
Loading
Loading