Skip to content

Commit

Permalink
Make ignore() work inside special functions
Browse files Browse the repository at this point in the history
loadd(), readd(), file_in(), file_out(), knitr_in()
  • Loading branch information
wlandau-lilly committed Jun 7, 2019
1 parent e0dfcbe commit 941c989
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 10 deletions.
1 change: 1 addition & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
18 changes: 14 additions & 4 deletions R/api-read.R
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
5 changes: 5 additions & 0 deletions R/preprocess-codeanalysis.R
Original file line number Diff line number Diff line change
Expand Up @@ -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"]))
Expand All @@ -110,26 +111,30 @@ 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)
ht_set(results$file_in, x)
}

analyze_file_out <- function(expr, results) {
expr <- ignore_ignore(expr)
x <- analyze_strings(expr[-1])
x <- file.path(x)
x <- encode_path(x)
ht_set(results$file_out, x)
}

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))
Expand Down
20 changes: 14 additions & 6 deletions man/readd.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

43 changes: 43 additions & 0 deletions tests/testthat/test-dependencies.R
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}
}
})

0 comments on commit 941c989

Please sign in to comment.