ropensci-books/drake

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.