Skip to content

Commit f809f0e

Browse files
authored
Merge pull request #634 from cmu-delphi/lcb/address-dist_quantiles-deprecation
Address `dist_quantiles()` deprecation
2 parents ca98924 + 4e00ac3 commit f809f0e

8 files changed

+73
-39
lines changed

DESCRIPTION

+3-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
Type: Package
22
Package: epiprocess
33
Title: Tools for basic signal processing in epidemiology
4-
Version: 0.11.0
4+
Version: 0.11.1
55
Authors@R: c(
66
person("Jacob", "Bien", role = "ctb"),
77
person("Logan", "Brooks", , "[email protected]", role = c("aut", "cre")),
@@ -66,12 +66,13 @@ Suggests:
6666
distributional,
6767
epidatr,
6868
epipredict,
69+
hardhat,
6970
here,
7071
knitr,
7172
outbreaks,
7273
readr,
7374
rmarkdown,
74-
testthat (>= 3.1.5),
75+
testthat,
7576
trendfilter,
7677
withr
7778
VignetteBuilder:

R/key_colnames.R

+2-2
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,8 @@ key_colnames.epi_df <- function(x, ...,
9191
if (!identical(other_keys, expected_other_keys)) {
9292
cli_abort(c(
9393
"The provided `other_keys` argument didn't match the `other_keys` of `x`",
94-
"*" = "`other_keys` was {format_chr_with_quotes(other_keys)}",
95-
"*" = "`expected_other_keys` was {format_chr_with_quotes(expected_other_keys)}",
94+
"*" = "`other_keys` was {format_chr_deparse(other_keys)}",
95+
"*" = "`expected_other_keys` was {format_chr_deparse(expected_other_keys)}",
9696
"i" = "If you know that `x` will always be an `epi_df` and
9797
resolve this discrepancy by adjusting the metadata of `x`, you
9898
shouldn't have to pass `other_keys =` here anymore,

R/methods-epi_archive.R

+2-1
Original file line numberDiff line numberDiff line change
@@ -450,7 +450,8 @@ epix_merge <- function(x, y,
450450
y_nonby_colnames <- setdiff(names(y_dt), by)
451451
if (length(intersect(x_nonby_colnames, y_nonby_colnames)) != 0L) {
452452
cli_abort("
453-
`x` and `y` DTs have overlapping non-by column names;
453+
`x` and `y` DTs both have measurement columns named
454+
{format_chr_with_quotes(intersect(x_nonby_colnames, y_nonby_colnames))};
454455
this is currently not supported; please manually fix up first:
455456
any overlapping columns that can are key-like should be
456457
incorporated into the key, and other columns should be renamed.

R/utils.R

+17-14
Original file line numberDiff line numberDiff line change
@@ -98,26 +98,29 @@ format_chr_deparse <- function(x) {
9898
paste(collapse = "", deparse(x))
9999
}
100100

101-
#' Format a character vector as a string via deparsing/quoting each
101+
#' Format each entry in a character vector via quoting; special replacement for length 0
102+
#'
103+
#' Performs no escaping within the strings; if you want something that reader
104+
#' could copy-paste to debug, look into `format_deparse` (note that this
105+
#' collapses into a single string).
106+
#'
107+
#' @param x chr; e.g., `colnames` of some data frame
108+
#' @param empty chr, likely string; what should be output if `x` is of length 0?
109+
#' @return chr; same `length` as `x` if `x` had nonzero length; value of `empty` otherwise
110+
#'
111+
#' @examples
112+
#' cli::cli_inform('{epiprocess:::format_chr_with_quotes("x")}')
113+
#' cli::cli_inform('{epiprocess:::format_chr_with_quotes(c("x","y"))}')
114+
#' nms <- c("x", "\"Total Cases\"")
115+
#' cli::cli_inform("{epiprocess:::format_chr_with_quotes(nms)}")
116+
#' cli::cli_inform("{epiprocess:::format_chr_with_quotes(character())}")
102117
#'
103-
#' @param x `chr`; e.g., `colnames` of some data frame
104-
#' @param empty string; what should be output if `x` is of length 0?
105-
#' @return string
106118
#' @keywords internal
107119
format_chr_with_quotes <- function(x, empty = "*none*") {
108120
if (length(x) == 0L) {
109121
empty
110122
} else {
111-
# Deparse to get quoted + escape-sequenced versions of varnames; collapse to
112-
# single line (assuming no newlines in `x`). Though if we hand this to cli
113-
# it may insert them (even in middle of quotes) while wrapping lines.
114-
deparsed_collapsed <- paste(collapse = "", deparse(x))
115-
if (length(x) == 1L) {
116-
deparsed_collapsed
117-
} else {
118-
# remove surrounding `c()`:
119-
substr(deparsed_collapsed, 3L, nchar(deparsed_collapsed) - 1L)
120-
}
123+
paste0('"', x, '"')
121124
}
122125
}
123126

man/format_chr_with_quotes.Rd

+15-5
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

tests/testthat.R

+2
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
library(testthat)
22
library(epiprocess)
33

4+
stopifnot(packageVersion("testthat") >= "3.1.5")
5+
46
test_check("epiprocess")

tests/testthat/test-compactify.R

+31-14
Original file line numberDiff line numberDiff line change
@@ -120,37 +120,54 @@ test_that("compactify does not alter the default clobberable and observed versio
120120
expect_identical(ea_true$versions_end, ea_false$versions_end)
121121
})
122122

123+
quantile_pred_once <- function(estimates_vec, levels_vec) {
124+
hardhat::quantile_pred(t(as.matrix(estimates_vec)), levels_vec)
125+
}
123126
test_that("compactify works on distributions", {
127+
skip("Until #611 is merged or hardhat/epipredict is patched")
124128
forecasts <- tibble(
125129
ahead = 2L,
126130
geo_value = "ak",
127131
target_end_date = as.Date("2020-01-19"),
128-
forecast_date = as.Date("2020-01-17") + 1:8,
132+
forecast_date = as.Date("2020-01-17") + 1:6,
129133
actual = 25,
130134
.pred_distn = c(
131-
epipredict::dist_quantiles(c(1, 5, 9), c(0.1, 0.5, 0.9)),
132-
epipredict::dist_quantiles(c(1, NA, 9), c(0.1, 0.5, 0.9)), # single NA in quantiles
133-
epipredict::dist_quantiles(c(NA, NA, NA), c(0.1, 0.5, 0.9)), # all NAs in quantiles
134-
distributional::dist_missing(1), # the actual `NA` for distributions
135-
epipredict::dist_quantiles(c(1, 5, 9), c(0.1, 0.5, 0.9)), # and back
136-
epipredict::dist_quantiles(c(3, 5, 9), c(0.1, 0.5, 0.9)), # change quantile
137-
epipredict::dist_quantiles(c(3, 5, 9), c(0.2, 0.5, 0.8)), # change level
138-
epipredict::dist_quantiles(c(3, 5, 9), c(0.2, 0.5, 0.8)) # LOCF
135+
quantile_pred_once(c(1, 5, 9), c(0.1, 0.5, 0.9)),
136+
quantile_pred_once(c(1, NA, 9), c(0.1, 0.5, 0.9)), # single NA in quantiles
137+
quantile_pred_once(c(NA, NA, NA), c(0.1, 0.5, 0.9)), # all NAs in quantiles
138+
quantile_pred_once(c(1, 5, 9), c(0.1, 0.5, 0.9)), # and back
139+
quantile_pred_once(c(3, 5, 9), c(0.1, 0.5, 0.9)), # change quantile
140+
quantile_pred_once(c(3, 5, 9), c(0.1, 0.5, 0.9)) # LOCF
139141
)
140142
)
141143
expect_equal(
142144
forecasts %>%
143-
as_epi_archive(
144-
other_keys = "ahead", time_value = target_end_date, version = forecast_date,
145-
compactify = TRUE
146-
) %>%
145+
as_epi_archive(other_keys = "ahead", time_value = target_end_date, version = forecast_date) %>%
147146
.$DT %>%
148147
as.data.frame() %>%
149148
as_tibble(),
150-
forecasts[-8, ] %>%
149+
forecasts[-6, ] %>%
151150
rename(time_value = target_end_date, version = forecast_date)
152151
)
153152
})
153+
test_that("epix_merge works with distributions", {
154+
skip("Until hardhat/epipredict is patched")
155+
forecasts1ea <- tibble(
156+
ahead = 2L,
157+
geo_value = "ak",
158+
target_end_date = as.Date("2020-01-19"),
159+
forecast_date = as.Date("2020-01-17") + 1,
160+
.pred_distn1 = quantile_pred_once(c(1, 5, 9), c(0.1, 0.5, 0.9))
161+
) %>% as_epi_archive(other_keys = "ahead", time_value = target_end_date, version = forecast_date)
162+
forecasts2ea <- tibble(
163+
ahead = 2L,
164+
geo_value = "ak",
165+
target_end_date = as.Date("2020-01-19"),
166+
forecast_date = as.Date("2020-01-17") + 2,
167+
.pred_distn2 = quantile_pred_once(c(2, 4, 8), c(0.1, 0.5, 0.9))
168+
) %>% as_epi_archive(other_keys = "ahead", time_value = target_end_date, version = forecast_date)
169+
forecasts12ea <- epix_merge(forecasts1ea, forecasts2ea, sync = "locf")
170+
})
154171

155172
test_that("Large compactify_abs_tol does not drop edf keys", {
156173
# several epikeytimes, each with a single version

tests/testthat/test-epix_merge.R

+1-1
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ test_that("epix_merge forbids and warns on metadata and naming issues", {
175175
as_epi_archive(tibble::tibble(geo_value = "ak", time_value = test_date, version = test_date + 1L, value = 1L)),
176176
as_epi_archive(tibble::tibble(geo_value = "ak", time_value = test_date, version = test_date + 1L, value = 2L))
177177
),
178-
regexp = "overlapping.*names"
178+
regexp = 'both have measurement columns named "value"'
179179
)
180180
})
181181

0 commit comments

Comments
 (0)