Skip to content

Commit 2902cc2

Browse files
committed
Detecting required/forbidden steps beforehand
1 parent a364518 commit 2902cc2

File tree

2 files changed

+39
-12
lines changed

2 files changed

+39
-12
lines changed

R/step_adjust_latency.R

+16-12
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@
7373
#' # step_adjust_latency(method = "extend_ahead") %>%
7474
#' step_epi_lag(death_rate, lag = c(0, 7, 14))
7575
#' r
76+
#' @importFrom recipes detect_step
7677
step_adjust_latency <-
7778
function(recipe,
7879
...,
@@ -98,6 +99,21 @@ step_adjust_latency <-
9899
i = "Use `tidyselect` methods to choose columns to lag."
99100
))
100101
}
102+
if ((method == "extend_ahead") && (!detect_step(recipe, "epi_ahead"))) {
103+
cli::cli_abort(
104+
"If `method` is {.val extend_ahead}, then a step
105+
must have already added an outcome."
106+
)
107+
} else if ((method == "extend_lags") && (!detect_step(recipe, "epi_lag"))) {
108+
cli::cli_abort(
109+
"If `method` is {.val extend_lags} or {.val locf}, then a step
110+
must have already added a predictor."
111+
)
112+
}
113+
if (detect_step(recipe, "naomit")) {
114+
cli::cli_abort("adjust_latency needs to occur before any `NA` removal,
115+
as columns may be moved around")
116+
}
101117

102118
method <- rlang::arg_match(method)
103119
terms_used <- recipes_eval_select(enquos(...), recipe$template, recipe$term_info)
@@ -164,20 +180,8 @@ step_adjust_latency_new <-
164180
}
165181

166182
# lags introduces max(lags) NA's after the max_time_value.
167-
# TODO all of the shifting happens before NA removal, which saves all the data I might possibly want; I should probably add a bit that makes sure this operation is happening before NA removal so data doesn't get dropped
168183
#' @export
169184
prep.step_adjust_latency <- function(x, training, info = NULL, ...) {
170-
if ((x$method == "extend_ahead") && (!("outcome" %in% info$role))) {
171-
cli::cli_abort(paste(
172-
"If `method` is {.val extend_ahead}, then a step ",
173-
"must have already added an outcome."
174-
))
175-
} else if (!("predictor" %in% info$role)) {
176-
cli::cli_abort(paste(
177-
"If `method` is {.val extend_lags} or {.val locf}, then a step ",
178-
"must have already added a predictor."
179-
))
180-
}
181185

182186
# get the columns used, even if it's all of them
183187
terms_used <- x$columns

tests/testthat/test-step_adjust_latency.R

+23
Original file line numberDiff line numberDiff line change
@@ -132,4 +132,27 @@ test_that("epi_adjust_latency extend_ahead uses the same adjustment when predict
132132

133133
test_that("epi_adjust_latency works for other time types", {})
134134

135+
test_that("epi_adjust_latency insist there's steps before it", {
136+
expect_error(
137+
r5 <- epi_recipe(x) %>%
138+
step_epi_ahead(death_rate, ahead = ahead) %>%
139+
step_adjust_latency(method = "extend_lags"),
140+
regexp = "extend_lags"
141+
)
142+
expect_error(
143+
r5 <- epi_recipe(x) %>%
144+
step_epi_lag(death_rate, lag = c(0, 6, 11)) %>%
145+
step_adjust_latency(method = "extend_ahead"),
146+
regexp = "extend_ahead"
147+
)
148+
})
149+
150+
test_that("epi_adjust_latency warns against removing NA's beforehand", {
151+
expect_error(r5 <- epi_recipe(x) %>%
152+
step_epi_lag(death_rate, lag = c(0, 6, 11)) %>%
153+
step_epi_lag(case_rate, lag = c(1, 5)) %>%
154+
step_epi_naomit() %>%
155+
step_adjust_latency(method = "extend_lags"),
156+
regexp = "adjust_latency needs to occur before any `NA` removal")
157+
})
135158
# todo check that epi_adjust_latency errors for nonsense `as_of`'s

0 commit comments

Comments
 (0)