Help users deal with empty dynamic sub-targets
wlandau opened this issue · 3 comments
From a conversation with @MilesMcBain: some dynamic targets in real life have inconsistent return values (some NULL
, some not). This can make it hard to know what to do the aggregated target.
library(drake)
f <- function(x) {
if (x > 2L) {
x
} else {
NULL
}
}
plan <- drake_plan(
x = seq_len(4L),
y = target(f(x), dynamic = map(x))
)
make(plan)
#> target x
#> dynamic y
#> subtarget y_0b3474bd
#> subtarget y_b2a5c9b8
#> subtarget y_71f311ad
#> subtarget y_98cf3c11
#> aggregate y
subtargets(y) # length 4
#> [1] "y_0b3474bd" "y_b2a5c9b8" "y_71f311ad" "y_98cf3c11"
readd(y) # length 2
#> [1] 3 4
readd(y, subtargets = seq_len(2))
#> NULL
Created on 2020-01-18 by the reprex package (v0.3.0)
At the very least, this deserves discussion in the manual.
We also talked about throwing warnings for NULL
values, but on reflection, I worry that it would overfit the motivating use case. NULL
values are not necessarily incorrect. And what about sub-targets with varying lengths? Again, it could be strange in some use cases, but it is not necessarily wrong.
Yeah I think I agree that a warning is tough call. You'd want the option to disable it if you went with that.
I also realised part of the reason it took me a bit to figure out what was going on is that it's hard to actually look at all the values of subtargets.
What if subtargets
returned a named list (key-value pairs)? List elems are nullable so you don't have the same issue as vectors.
I like the idea of an optional named list. I think the smoothest way to implement that is with a logical subtarget_list
argument to loadd()
and readd()
.
Implemented in ropensci/drake@94d2896.