Skip to content

Commit b90916d

Browse files
authored
Merge pull request #101 from dajmcdon/km-compactify_rectify
Km compactify rectify
2 parents 43a90d4 + 2a79c08 commit b90916d

26 files changed

+2034
-264
lines changed

DESCRIPTION

+3-1
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,9 @@ Suggests:
4646
knitr,
4747
outbreaks,
4848
rmarkdown,
49-
testthat (>= 3.0.0)
49+
testthat (>= 3.0.0),
50+
waldo (>= 0.3.1),
51+
withr
5052
VignetteBuilder:
5153
knitr
5254
Remotes:

NAMESPACE

+11-2
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ S3method(filter,epi_df)
1111
S3method(group_by,epi_df)
1212
S3method(group_modify,epi_df)
1313
S3method(mutate,epi_df)
14+
S3method(next_after,Date)
15+
S3method(next_after,integer)
1416
S3method(print,epi_df)
1517
S3method(relocate,epi_df)
1618
S3method(rename,epi_df)
@@ -19,6 +21,7 @@ S3method(summary,epi_df)
1921
S3method(ungroup,epi_df)
2022
S3method(unnest,epi_df)
2123
export("%>%")
24+
export(archive_cases_dv_subset)
2225
export(arrange)
2326
export(as_epi_archive)
2427
export(as_epi_df)
@@ -38,19 +41,23 @@ export(group_modify)
3841
export(growth_rate)
3942
export(is_epi_archive)
4043
export(is_epi_df)
44+
export(max_version_with_row_in)
4145
export(mutate)
4246
export(new_epi_df)
47+
export(next_after)
4348
export(relocate)
4449
export(rename)
4550
export(slice)
4651
export(ungroup)
4752
export(unnest)
4853
importFrom(R6,R6Class)
54+
importFrom(data.table,":=")
55+
importFrom(data.table,address)
4956
importFrom(data.table,as.data.table)
5057
importFrom(data.table,between)
58+
importFrom(data.table,copy)
5159
importFrom(data.table,key)
52-
importFrom(data.table,merge.data.table)
53-
importFrom(data.table,nafill)
60+
importFrom(data.table,set)
5461
importFrom(data.table,setkeyv)
5562
importFrom(dplyr,arrange)
5663
importFrom(dplyr,filter)
@@ -69,9 +76,11 @@ importFrom(rlang,"!!!")
6976
importFrom(rlang,"!!")
7077
importFrom(rlang,.data)
7178
importFrom(rlang,.env)
79+
importFrom(rlang,arg_match)
7280
importFrom(rlang,enquo)
7381
importFrom(rlang,enquos)
7482
importFrom(rlang,is_quosure)
83+
importFrom(rlang,quo_is_missing)
7584
importFrom(rlang,sym)
7685
importFrom(rlang,syms)
7786
importFrom(stats,cor)

R/archive.R

+412-75
Large diffs are not rendered by default.

R/data.R

+122-1
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,129 @@
6767
#' * \href{https://cmu-delphi.github.io/delphi-epidata/api/covidcast-signals/doctor-visits.html}{From the COVIDcast Doctor Visits API}: The signal `percent_cli` is taken directly from the API without changes.
6868
#' * \href{https://cmu-delphi.github.io/delphi-epidata/api/covidcast-signals/jhu-csse.html}{From the COVIDcast Epidata API}: `case_rate_7d_av` is taken directly from the JHU CSSE \href{https://github.com/CSSEGISandData/COVID-19}{COVID-19 GitHub repository} without changes. The 7-day average signals are computed by Delphi by calculating moving averages of the preceding 7 days, so the signal for June 7 is the average of the underlying data for June 1 through 7, inclusive.
6969
#' * Furthermore, the data is a subset of the full dataset, the signal names slightly altered, and formatted into a tibble.
70+
#'
71+
#' @export
7072
"archive_cases_dv_subset"
7173

74+
#' Detect whether `pkgload` is unregistering a package (with some unlikely false positives)
75+
#'
76+
#' More precisely, detects the presence of a call to an `unregister` or
77+
#' `unregister_namespace` function from any package in the indicated part of the
78+
#' function call stack.
79+
#'
80+
#' @param parent_n optional, single non-`NA` non-negative integer; how many
81+
#' "parent"/"ancestor" calls should we skip inspecting? Default of `0L` will
82+
#' check everything up to, but not including the call to this function. If
83+
#' building wrappers or utilities around this function it may be useful to use
84+
#' this default to ignore those wrappers, especially if they might trigger
85+
#' false positives now or in some future version of this function with a looser
86+
#' function name test.
87+
#'
88+
#' @return Boolean
89+
#'
90+
#' @noRd
91+
some_package_is_being_unregistered = function(parent_n = 0L) {
92+
calls = sys.calls()
93+
# `calls` will include the call to this function; strip out this call plus
94+
# `parent_n` additional requested calls to make it like we're reasoning about
95+
# the desired call. This could prevent potential false positives from
96+
# triggering if, in a later version, we decide to loosen the `call_name`
97+
# checks below to something that would be `TRUE` for the name of this function
98+
# or one of the undesired call ancestors.
99+
calls_to_inspect = utils::head(calls, n = -(parent_n + 1L))
100+
# Note that `utils::head(sys.calls(), n=-1L)` isn't equivalent, due to lazy
101+
# argument evaluation. Note that copy-pasting the body of this function
102+
# without this `utils::head` operation isn't always equivalent to calling it;
103+
# e.g., within the `value` argument of a package-level `delayedAssign`,
104+
# `sys.calls()` will return `NULL` is some or all cases, including when its
105+
# evaluation has been triggered via `unregister`.
106+
simple_call_names = purrr::map_chr(calls_to_inspect, function(call) {
107+
maybe_simple_call_name = rlang::call_name(call)
108+
if (is.null(maybe_simple_call_name)) NA_character_ else maybe_simple_call_name
109+
})
110+
# `pkgload::unregister` is an (the?) exported function that forces
111+
# package-level promises, while `pkgload:::unregister_namespace` is the
112+
# internal function that does this package-level promise. Check for both just
113+
# in case there's another exported function that calls `unregister_namespace`
114+
# or other `pkgload` versions don't use the `unregister_namespace` internal.
115+
# (Note that `NA_character_ %in% <table not containing NA>` is `FALSE` rather
116+
# than `NA`, giving the desired semantics and avoiding potential `NA`s in the
117+
# argument to `any`.)
118+
any(simple_call_names %in% c("unregister", "unregister_namespace"))
119+
}
120+
121+
#' [`base::delayedAssign`] with [`pkgload::unregister`] awareness, injection support
122+
#'
123+
#' Provides better feedback on errors during promise evaluation while a package
124+
#' is being unregistered, to help package developers escape from a situation
125+
#' where a buggy promise prevents package reloading. Also provide `rlang`
126+
#' injection support (like [`rlang::env_bind_lazy`]). The call stack will look
127+
#' different than when using `delayedAssign` directly.
128+
#'
129+
#' @noRd
130+
delayed_assign_with_unregister_awareness = function(x, value,
131+
eval.env = rlang::caller_env(),
132+
assign.env = rlang::caller_env()) {
133+
value_quosure = rlang::as_quosure(rlang::enexpr(value), eval.env)
134+
this_env = environment()
135+
delayedAssign(x, eval.env = this_env, assign.env = assign.env, value = {
136+
if (some_package_is_being_unregistered()) {
137+
withCallingHandlers(
138+
# `rlang::eval_tidy(value_quosure)` is shorter and would sort of work,
139+
# but doesn't give the same `ls`, `rm`, and top-level `<-` behavior as
140+
# we'd have with `delayedAssign`; it doesn't seem to actually evaluate
141+
# quosure's expr in the quosure's env. Using `rlang::eval_bare` instead
142+
# seems to do the trick. (We also could have just used a `value_expr`
143+
# and `eval.env` together rather than introducing `value_quosure` at
144+
# all.)
145+
rlang::eval_bare(rlang::quo_get_expr(value_quosure), rlang::quo_get_env(value_quosure)),
146+
error = function(err) {
147+
Abort(paste("An error was raised while attempting to evaluate a promise",
148+
"(prepared with `delayed_assign_with_unregister_awareness`)",
149+
"while an `unregister` or `unregister_namespace` call",
150+
"was being evaluated.",
151+
"This can happen, for example, when `devtools::load_all`",
152+
"reloads a package that contains a buggy promise,",
153+
"because reloading can cause old package-level promises to",
154+
"be forced via `pkgload::unregister` and",
155+
"`pkgload:::unregister_namespace`, due to",
156+
"https://github.com/r-lib/pkgload/pull/157.",
157+
"If this is the current situation, you might be able to",
158+
"be successfully reload the package again after",
159+
"`unloadNamespace`-ing it (but this situation will",
160+
"keep re-occurring every other `devtools::load`",
161+
"and every `devtools:document` until the bug or situation",
162+
"generating the promise's error has been resolved)."
163+
),
164+
class = "epiprocess__promise_evaluation_error_during_unregister",
165+
parent = err)
166+
})
167+
} else {
168+
rlang::eval_bare(rlang::quo_get_expr(value_quosure), rlang::quo_get_env(value_quosure))
169+
}
170+
})
171+
}
172+
173+
# Like normal data objects, set `archive_cases_dv_subset` up as a promise, so it
174+
# doesn't take unnecessary space before it's evaluated. This also avoids a need
175+
# for @include tags. However, this pattern will use unnecessary space after this
176+
# promise is evaluated, because `as_epi_archive` clones `archive_cases_dv_subset_dt`
177+
# and `archive_cases_dv_subset_dt` will stick around along with `archive_cases_dv_subset`
178+
# after they have been evaluated. We may want to add an option to avoid cloning
179+
# in `as_epi_archive` and make use of it here. But we may also want to change
180+
# this into an active binding that clones every time, unless we can hide the
181+
# `DT` field from the user (make it non-`public` in general) or make it
182+
# read-only (in this specific case), so that the user cannot modify the `DT`
183+
# here and potentially mess up examples that they refer to later on.
184+
#
185+
# During development, note that reloading the package and re-evaluating this
186+
# promise should prepare the archive from the DT using any changes that have
187+
# been made to `as_epi_archive`; however, if earlier, any field of
188+
# `archive_cases_dv_subset` was modified using `<-`, a global environment
189+
# binding may have been created with the same name as the package promise, and
190+
# this binding will stick around even when the package is reloaded, and will
191+
# need to be `rm`-d to easily access the refreshed package promise.
192+
delayed_assign_with_unregister_awareness("archive_cases_dv_subset", as_epi_archive(archive_cases_dv_subset_dt, compactify=FALSE))
72193

73194
#' Subset of JHU daily cases from California and Florida
74195
#'
@@ -120,4 +241,4 @@
120241
#' Modifications:
121242
#' * \href{https://cmu-delphi.github.io/delphi-epidata/api/covidcast-signals/jhu-csse.html}{From the COVIDcast Epidata API}: These signals are taken directly from the JHU CSSE \href{https://github.com/CSSEGISandData/COVID-19}{COVID-19 GitHub repository} without changes. The 7-day average signals are computed by Delphi by calculating moving averages of the preceding 7 days, so the signal for June 7 is the average of the underlying data for June 1 through 7, inclusive.
122243
#' * Furthermore, the data has been limited to a very small number of rows, the signal names slightly altered, and formatted into a tibble.
123-
"jhu_csse_county_level_subset"
244+
"jhu_csse_county_level_subset"

0 commit comments

Comments
 (0)