From 941c98940db947d807c24152e3d32adf6a9f6e74 Mon Sep 17 00:00:00 2001 From: wlandau Date: Thu, 6 Jun 2019 22:48:02 -0400 Subject: [PATCH] Make ignore() work inside special functions loadd(), readd(), file_in(), file_out(), knitr_in() --- NEWS.md | 1 + R/api-read.R | 18 ++++++++++--- R/preprocess-codeanalysis.R | 5 ++++ man/readd.Rd | 20 +++++++++----- tests/testthat/test-dependencies.R | 43 ++++++++++++++++++++++++++++++ 5 files changed, 77 insertions(+), 10 deletions(-) diff --git a/NEWS.md b/NEWS.md index 3dddf6b23..4e2383d4d 100644 --- a/NEWS.md +++ b/NEWS.md @@ -18,6 +18,7 @@ These changes are technically breaking changes, but they should only affect adva - Repair `expose_imports()`: do not do the `environment<-` trick unless the object is a non-primitive function. - Use different static analyses of `assign()` vs `delayedAssign()`. - Fix a superfluous code analysis warning incurred by multiple `file_in()` files and other strings ([#896](https://github.com/ropensci/drake/issues/896)). +- Make `ignore()` work inside `loadd()`, `readd()`, `file_in()`, `file_out()`, and `knitr_in()`. ## New features diff --git a/R/api-read.R b/R/api-read.R index 9c5265d9a..d2a1cc682 100644 --- a/R/api-read.R +++ b/R/api-read.R @@ -26,10 +26,20 @@ #' and then treat those loaded targets as dependencies. #' That way, [make()] will automatically (re)run the report if those #' dependencies change. -#' @note Please do not put calls to [loadd()] or [readd()] inside -#' your custom (imported) functions or the commands in your [drake_plan()]. -#' This creates confusion inside [make()], which has its own ways of -#' interacting with the cache. +#' 3. If you are using `make(memory_strategy = "none")` +#' or `make(memory_strategy = "unload")`, +#' [loadd()] and [readd()] can manually load dependencies +#' into memory for the target that is being built. +#' If you do this, you must carefully inspect [deps_target()] +#' and [vis_drake_graph()] before running [make()] +#' to be sure the dependency relationships among targets +#' are correct. If you do not wish to incur extra dependencies +#' with [loadd()] or [readd()], you will need to use [ignore()], +#' e.g. `drake_plan(x = 1, y = ignore(readd(x)))` or +#' `drake_plan(x = 1, y = readd(ignore("x"), character_only = TRUE))`. +#' Compare those plans to `drake_plan(x = 1, y = readd(x))` +#' and `drake_plan(x = 1, y = readd("x", character_only = TRUE))` +#' using [vis_drake_graph()] and [deps_target()]. #' @seealso [cached()], [drake_plan()], [make()] #' @export #' @return The cached value of the `target`. diff --git a/R/preprocess-codeanalysis.R b/R/preprocess-codeanalysis.R index a37b4b593..e3eee2b52 100644 --- a/R/preprocess-codeanalysis.R +++ b/R/preprocess-codeanalysis.R @@ -98,6 +98,7 @@ analyze_delayed_assign <- function(expr, results, locals, allowed_globals) { } analyze_loadd <- function(expr, results) { + expr <- ignore_ignore(expr) expr <- match.call(drake::loadd, as.call(expr)) expr <- expr[-1] ht_set(results$loadd, analyze_strings(expr["list"])) @@ -110,12 +111,14 @@ analyze_loadd <- function(expr, results) { } analyze_readd <- function(expr, results, allowed_globals) { + expr <- ignore_ignore(expr) expr <- match.call(drake::readd, as.call(expr)) ht_set(results$readd, analyze_strings(expr["target"])) ht_set(results$readd, safe_all_vars(expr["target"])) } analyze_file_in <- function(expr, results) { + expr <- ignore_ignore(expr) x <- analyze_strings(expr[-1]) x <- file.path(x) x <- encode_path(x) @@ -123,6 +126,7 @@ analyze_file_in <- function(expr, results) { } analyze_file_out <- function(expr, results) { + expr <- ignore_ignore(expr) x <- analyze_strings(expr[-1]) x <- file.path(x) x <- encode_path(x) @@ -130,6 +134,7 @@ analyze_file_out <- function(expr, results) { } analyze_knitr_in <- function(expr, results) { + expr <- ignore_ignore(expr) files <- analyze_strings(expr[-1]) lapply(files, analyze_knitr_file, results = results) ht_set(results$knitr_in, encode_path(files)) diff --git a/man/readd.Rd b/man/readd.Rd index 43e414543..41c3f1456 100644 --- a/man/readd.Rd +++ b/man/readd.Rd @@ -139,14 +139,22 @@ calls to \code{loadd()} and \code{readd()} in active code chunks, and then treat those loaded targets as dependencies. That way, \code{\link[=make]{make()}} will automatically (re)run the report if those dependencies change. +\item If you are using \code{make(memory_strategy = "none")} +or \code{make(memory_strategy = "unload")}, +\code{\link[=loadd]{loadd()}} and \code{\link[=readd]{readd()}} can manually load dependencies +into memory for the target that is being built. +If you do this, you must carefully inspect \code{\link[=deps_target]{deps_target()}} +and \code{\link[=vis_drake_graph]{vis_drake_graph()}} before running \code{\link[=make]{make()}} +to be sure the dependency relationships among targets +are correct. If you do not wish to incur extra dependencies +with \code{\link[=loadd]{loadd()}} or \code{\link[=readd]{readd()}}, you will need to use \code{\link[=ignore]{ignore()}}, +e.g. \code{drake_plan(x = 1, y = ignore(readd(x)))} or +\code{drake_plan(x = 1, y = readd(ignore("x"), character_only = TRUE))}. +Compare those plans to \code{drake_plan(x = 1, y = readd(x))} +and \code{drake_plan(x = 1, y = readd("x", character_only = TRUE))} +using \code{\link[=vis_drake_graph]{vis_drake_graph()}} and \code{\link[=deps_target]{deps_target()}}. } } -\note{ -Please do not put calls to \code{\link[=loadd]{loadd()}} or \code{\link[=readd]{readd()}} inside -your custom (imported) functions or the commands in your \code{\link[=drake_plan]{drake_plan()}}. -This creates confusion inside \code{\link[=make]{make()}}, which has its own ways of -interacting with the cache. -} \examples{ \dontrun{ isolate_example("Quarantine side effects.", { diff --git a/tests/testthat/test-dependencies.R b/tests/testthat/test-dependencies.R index a94440940..40011e5c6 100644 --- a/tests/testthat/test-dependencies.R +++ b/tests/testthat/test-dependencies.R @@ -470,3 +470,46 @@ test_with_dir("ignore() in imported functions", { config <- drake_config(plan, cache = cache) expect_equal(justbuilt(config), "x") }) + +test_with_dir("ignore() inside special functions", { + plan <- drake_plan( + a = 1, + b1 = readd(a), + b2 = readd("a"), + b3 = ignore(readd(a)), + b4 = readd(ignore(a)), + b5 = readd(ignore("a")), + c1 = loadd(a), + c2 = loadd("a"), + c3 = ignore(loadd(a)), + c4 = loadd(ignore(a)), + c5 = loadd(ignore("a")), + d1 = file_in("a"), + d2 = file_in("a"), + d3 = ignore(file_in("a")), + d4 = file_in(ignore("a")), + d5 = file_in(ignore("a")), + e1 = file_out("a"), + e2 = file_out("a"), + e3 = ignore(file_out("a")), + e4 = file_out(ignore("a")), + e5 = file_out(ignore("a")), + f1 = knitr_in("a"), + f2 = knitr_in("a"), + f3 = ignore(knitr_in("a")), + f4 = knitr_in(ignore("a")), + f5 = knitr_in(ignore("a")) + ) + suppressWarnings(config <- drake_config(plan)) + for (x in letters[2:6]) { + for (y in 1:5) { + target <- paste0(x, y) + deps <- deps_target(target, config, character_only = TRUE)$name + if (y < 3) { + expect_equal(deps, "a") + } else { + expect_equal(deps, character(0)) + } + } + } +})