-
Notifications
You must be signed in to change notification settings - Fork 8
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
Add way to refer to ref_time_value
within f
arg to epix_slide
#170
Comments
This is already possible in the current interface, as |
Here are two possible ways to be able to access
|
On further discussion with Logan, we'd prefer to match There are additional This looks pretty complicated to implement and would also depend on non-exported
|
We could support use of |
On further discussion with @ryantibs, plan to implement one of the easy approaches above to make the |
epix_slide
parameters, output #163, but the desired approach here is more settled, and any renaming decided on there could be done later.epi_slide
, but this is more important + easier to implement: Add way to refer toref_time_value
withinf
arg toepi_slide
#171n
inepi[x]_slide
withbefore
,after
#161.Currently,
epix_slide
does not make the currentref_time_value
(t
in the current code) available to the suppliedf
. This is a problem, e.g., when forecasting, where the targets forref_time_value
should be observations atref_time_value + aheads
, but there is data reporting latency, so data are only available fortime_value
s throughref_time_value - latency
. We are left with either incorrectly pretending thatmax(.x$time_value)
is the same asref_time_value
, or making some guess or inconvenient calculation of the latency, so we can sayref_time_value = max(.x$time_value) + latency_guess_or_calculation
.Instead, we should make
ref_time_value
available tof
. Approaches to passing:.x
: doesn'twork, we don'tprovide an epi_df. [but same issue of potentially being overlooked].x
: works across all forms off
, but is easily overlooked.Current epi[x]_slide
f
functions take:Since
epi_slide
implementation might be trickier and take longer, it makes sense to makeepix_slide
'sf
functions take the following:(If instead we implement the corresponding epi_slide change simultaneously, we could place the ref_time_value before the group key if we felt it made more sense. I don't think one approach is dominant in this case, though. Often we group by geo, so group key followed by ref_time_value is sort of a geo value followed by sort of a time value, which seems natural given how we order epi_df columns. However, sometimes we'll group by nothing, group key followed by ref_time_value would be something useless followed by something useful, which is a bit unnatural.)
--- Implementation ideas: ---
Passing ref_time_value as a third arg when
f
is a function may require:f
must take 3 positional args + user's custom args.f
follows this format. See Validatef
function inepi[x]_slide
; give better feedback if doesn't take enough args #168.comp_one_group
tofunction(.data_group, g, <rest of the current args>)
and callingf(.data_group, g, ref_time_value=t, ...)
.Passing
ref_time_value
whenf
is a formula might require:..3
orref_time_value
rlang::as_data_{pronoun,mask}
+ otherrlang
stuff?), still usingpurrr::map_dfr
.purrr::map_dfr
with more freedom.Passing
ref_time_value
whenf
is missing &...
contains summarize-like operations:f
to a new child of itself, and bindingref_time_value
within that child environment. (Careful, check whether we need to do something to copyf
to avoid modifying the realf
's environment.)cur_group()
work and mimic it with, e.g.,cur_ref_time_value()
.]The text was updated successfully, but these errors were encountered: