From 94d289639574dc377a067ce463e04509b446c6f6 Mon Sep 17 00:00:00 2001 From: wlandau Date: Sun, 19 Jan 2020 09:46:09 -0500 Subject: [PATCH] Fix #1139 --- NEWS.md | 1 + R/cache.R | 66 ++++++++++++++++++++++++++------- R/manage_memory.R | 2 +- man/readd.Rd | 12 +++++- tests/testthat/test-9-dynamic.R | 28 ++++++++++++++ 5 files changed, 92 insertions(+), 17 deletions(-) diff --git a/NEWS.md b/NEWS.md index 0c87b973f..d443cb058 100644 --- a/NEWS.md +++ b/NEWS.md @@ -10,6 +10,7 @@ - Add a new `format` argument to `make()`, an optional custom storage format for targets without an explicit `target(format = ...)` in the plan (#1124). - Add a new `lock_cache` argument to `make()` to optionally suppress cache locking (#1129). (It can be annoying to interrupt `make()` repeatedly and unlock the cache manually every time.) - Add new functions `cancel()` and `cancel_if()` function to cancel targets mid-build (#1131). +- Add a new `subtarget_list` argument to `loadd()` and `readd()` to optionally load a dynamic target as a list of sub-targets (#1139, @MilesMcBain). ## Enhancements diff --git a/R/cache.R b/R/cache.R index f8948e4c7..207e33d27 100644 --- a/R/cache.R +++ b/R/cache.R @@ -66,6 +66,11 @@ #' to retrieve with the `subtargets` argument. For example, #' `readd(x, subtargets = seq_len(3))` only retrieves the #' first 3 sub-targets of dynamic target `x`. +#' @param subtarget_list Logical, for dynamic targets only. +#' If `TRUE`, the dynamic target is loaded as a named +#' list of sub-target values. If `FALSE`, `drake` +#' attempts to concatenate the sub-targets with `vctrs::vec_c()` +#' (and returns an unnamed list if such concatenation is not possible). #' @examples #' \dontrun{ #' isolate_example("Quarantine side effects.", { @@ -89,7 +94,8 @@ readd <- function( namespace = NULL, verbose = 1L, show_source = FALSE, - subtargets = NULL + subtargets = NULL, + subtarget_list = FALSE ) { deprecate_search(search) # if the cache is null after trying drake_cache: @@ -115,7 +121,7 @@ readd <- function( namespace = namespace, use_cache = FALSE ) - get_subtargets(value, cache, subtargets) + get_subtargets(value, target, cache, subtargets, subtarget_list) } #' @title Load one or more targets or imports from the drake cache. @@ -225,7 +231,8 @@ loadd <- function( show_source = FALSE, tidyselect = !deps, config = NULL, - subtargets = NULL + subtargets = NULL, + subtarget_list = FALSE ) { deprecate_search(search) deprecate_arg(graph, "graph") # 2019-01-04 # nolint @@ -279,17 +286,25 @@ loadd <- function( load_subtargets, cache = cache, envir = envir, - subtargets = subtargets + subtargets = subtargets, + subtarget_list = subtarget_list ) invisible() } -load_subtargets <- function(target, cache, envir, subtargets) { +load_subtargets <- function(target, cache, envir, subtargets, subtarget_list) { hashes <- get(target, envir = envir, inherits = FALSE) - load_targets_impl(hashes, target, cache, envir, subtargets) + load_targets_impl(hashes, target, cache, envir, subtargets, subtarget_list) } -load_targets_impl <- function(hashes, target, cache, envir, subtargets) { +load_targets_impl <- function( + hashes, + target, + cache, + envir, + subtargets, + subtarget_list +) { UseMethod("load_targets_impl") } @@ -298,9 +313,10 @@ load_targets_impl.drake_dynamic <- function( # nolint target, cache, envir, - subtargets + subtargets, + subtarget_list ) { - value <- get_subtargets(hashes, cache, subtargets) + value <- get_subtargets(hashes, target, cache, subtargets, subtarget_list) rm(list = target, envir = envir, inherits = FALSE) assign(target, value, envir = envir, inherits = FALSE) } @@ -310,24 +326,46 @@ load_targets_impl.default <- function( target, cache, envir, - subtargets + subtargets, + subtarget_list ) { NULL } -get_subtargets <- function(hashes, cache, subtargets) { +get_subtargets <- function(hashes, target, cache, subtargets, subtarget_list) { UseMethod("get_subtargets") } -get_subtargets.drake_dynamic <- function(hashes, cache, subtargets) { +get_subtargets.drake_dynamic <- function( + hashes, + target, + cache, + subtargets, + subtarget_list +) { if (!is.null(subtargets)) { hashes <- hashes[subtargets] } out <- lapply(hashes, cache$get_value, use_cache = FALSE) - do.call(safe_vec_c, out) + if (subtarget_list) { + keys <- cache$get(target, namespace = "meta")$subtargets + if (!is.null(subtargets)) { + keys <- keys[subtargets] + } + names(out) <- keys + } else { + out <- do.call(safe_vec_c, out) + } + out } -get_subtargets.default <- function(hashes, cache, subtargets) { +get_subtargets.default <- function( + hashes, + target, + cache, + subtargets, + subtarget_list +) { hashes } diff --git a/R/manage_memory.R b/R/manage_memory.R index 1870b39d5..0af5493d7 100644 --- a/R/manage_memory.R +++ b/R/manage_memory.R @@ -354,6 +354,6 @@ sync_envir_dynamic <- function(target, config) { sync_dynamic_whole <- function(target, config) { hashes <- get(target, envir = config$envir_targets, inherits = FALSE) - value <- get_subtargets(hashes, config$cache, NULL) + value <- get_subtargets(hashes, target, config$cache, NULL, FALSE) assign(target, value, envir = config$envir_dynamic, inherits = FALSE) } diff --git a/man/readd.Rd b/man/readd.Rd index 21b666641..9fc910b0e 100644 --- a/man/readd.Rd +++ b/man/readd.Rd @@ -15,7 +15,8 @@ readd( namespace = NULL, verbose = 1L, show_source = FALSE, - subtargets = NULL + subtargets = NULL, + subtarget_list = FALSE ) loadd( @@ -36,7 +37,8 @@ loadd( show_source = FALSE, tidyselect = !deps, config = NULL, - subtargets = NULL + subtargets = NULL, + subtarget_list = FALSE ) } \arguments{ @@ -73,6 +75,12 @@ to retrieve with the \code{subtargets} argument. For example, \code{readd(x, subtargets = seq_len(3))} only retrieves the first 3 sub-targets of dynamic target \code{x}.} +\item{subtarget_list}{Logical, for dynamic targets only. +If \code{TRUE}, the dynamic target is loaded as a named +list of sub-target values. If \code{FALSE}, \code{drake} +attempts to concatenate the sub-targets with \code{vctrs::vec_c()} +(and returns an unnamed list if such concatenation is not possible).} + \item{...}{Targets to load from the cache: as names (symbols) or character strings. If the \code{tidyselect} package is installed, you can also supply \code{dplyr}-style \code{tidyselect} diff --git a/tests/testthat/test-9-dynamic.R b/tests/testthat/test-9-dynamic.R index e1558055c..0647e3222 100644 --- a/tests/testthat/test-9-dynamic.R +++ b/tests/testthat/test-9-dynamic.R @@ -1239,6 +1239,34 @@ test_with_dir("dynamic loadd() and readd()", { } }) +test_with_dir("dyn loadd/readd with NULL subtargets (#1139)", { + skip_on_cran() + 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) + keys <- subtargets(y) + out <- readd(y, subtarget_list = TRUE) + exp <- list(NULL, NULL, 3L, 4L) + names(exp) <- keys + expect_equal(out, exp) + out <- readd(y, subtarget_list = TRUE, subtargets = c(2L, 3L)) + expect_equal(out, exp[c(2L, 3L)]) + e <- new.env(parent = emptyenv()) + loadd(y, envir = e, subtarget_list = TRUE) + expect_equal(e$y, exp) + loadd(y, envir = e, subtarget_list = TRUE, subtargets = c(2L, 3L)) + expect_equal(e$y, exp[c(2L, 3L)]) +}) + test_with_dir("dynamic hpc", { skip_on_cran() skip_on_os("windows")