make(session = callr::r_vanilla)

[wlandau]

Summary

Adds a session argument to make() and drake_config() to optionally tell make_with_config() to operate in a new, fresh, isolated master session. Example with reprex:

library(drake)
#> Warning: replacing previous import 'graph::addNode' by 'XML::addNode' when
#> loading 'CodeDepends'
#> Warning: replacing previous import 'graph::plot' by 'graphics::plot' when
#> loading 'CodeDepends'
clean(destroy = TRUE)
load_basic_example()
make(my_plan, session = callr::r_vanilla)
outdated(drake_config(my_plan))
#> character(0)

Benefits:

• Enhanced reproducibility.
• A more thorough solution to the problem Symbols loaded after make() #253 and Consider building targets in a separate environment. #296 were trying to solve.

Drawbacks:

• Extra overhead to set up the new session.
• Extra overhead and clutter to copy the imported globals to the new session.
• I needed to set libpath to .libPaths(), so reproducibility is not perfect.
• To make sure this feature gets tested correctly, I needed to temporarily alter the global environment in a unit test. Afterwards, I do try to remove any objects created during make(), but this is still bad form.

cc @rkrug, @AlexAxthelm, @krlmlr.

GitHub issues fixed

• Ref: Option to make targets in a new session #333

Checklist

• I have read drake's code of conduct, and I agree to follow its rules.
• I have read the guidelines for contributing.
• I have listed any substantial changes in the development news.
• I have added testthat unit tests to tests/testthat to confirm that any new features or functionality work correctly.
• I have tested this pull request locally with devtools::check()
• This pull request is ready for review.
• I think this pull request is ready to merge.

[codecov-io]

Codecov Report

Merging #346 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master   #346   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          66     66           
  Lines        5246   5266   +20     
=====================================
+ Hits         5246   5266   +20

Impacted Files	Coverage Δ
R/make.R	100% <100%> (ø)	⬆️
R/distributed.R	100% <100%> (ø)	⬆️
R/config.R	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d616b88...a7547ce. Read the comment docs.

[rkrug]

Mo objections - it looks fine, although I don't have any time to test it. Will do that beginning of next week.

[rkrug]

Thinking about it - would it be necessary to also use the session in the generation of graphs? The dependency graph is influenced by the environment (see #335 for problem this can cause)?

Must admit, havern't looked at the code yet...

[wlandau]

Unfortunately, we really do need the original session to build the graph. That way, we know which global imports we need to transfer to the new session. The alternative is just to copy over everything from globalenv(), which is not good for reproducibility. If you really want to avoid to avoid things from the global environment, you can define your functions in your own separate environment and pass it to make().

library(drake)
#> Warning: replacing previous import 'graph::addNode' by 'XML::addNode' when
#> loading 'CodeDepends'
#> Warning: replacing previous import 'graph::plot' by 'graphics::plot' when
#> loading 'CodeDepends'
clean(destroy = TRUE)
envir <- new.env(parent = baseenv())
evalq({
  f <- function(x) {
    g(x) + 1
  }
  g <- function(x) {
    x + 1
  }
}, envir = envir)
plan <- drake_plan(y = f(1))
make(plan, envir = envir, session = callr::r_vanilla)
readd(y)
#> [1] 3

[rkrug]

But this only impacts on the graph, or also the functioning of the compilation of the targets - right? So this is not a serious limitation for the repoducability of the targets / make process, but only for the dependency graph?

[wlandau]

Sorry, I am not sure what you mean.