Deprecate expose_imports()

NEWS.md

@@ -17,7 +17,7 @@
 
 ## Enhancements
 
-* Document a pitfall in `expose_imports()` (#1286, @mvarewyck).
+* Deprecate `expose_imports()` in favor of `make(envir = getNamespace("yourPackage")` (#1286, @mvarewyck).
 
 # Version 7.12.2
 

R/deprecated.R

@@ -2685,3 +2685,103 @@ progress <- function(
   )
   out[out$progress %in% progress,, drop = FALSE] # nolint
 }
+
+#' @title Deprecated: expose package functions and objects for analysis with drake
+#' \lifecycle{deprecated}
+#' @description Deprecated on 2020-06-24.
+#' @keywords internal
+#' @details Deprecated. This function assigns the objects and functions
+#'   from the package environment to the user's environment (usually global)
+#'   so `drake` can watch them for changes. This used to be the standard
+#'   way to make `drake` compatible with workflows implememented as custom
+#'   analysis packages. Now, the recommendation is to supply
+#'   `getNamespace("yourPackage")` to the `envir` argument of [make()]
+#'   and friends. Read <https://github.com/ropensci/drake/issues/1286>,
+#'   especially <https://github.com/ropensci/drake/issues/1286#issuecomment-649088321>, # nolint
+#'   for details.
+#' @export
+#' @return The environment that the exposed imports are loaded into.
+#'   Defaults to your R workspace.
+#' @param package Name of the package, either a symbol or a string,
+#'   depending on `character_only`.
+#' @param character_only Logical, whether to interpret `package`
+#'   as a character string or a symbol (quoted vs unquoted).
+#' @param envir Environment to load the exposed package imports.
+#'   You will later pass this `envir` to [make()].
+#' @param jobs Number of parallel jobs for the parallel processing
+#'   of the imports.
+#' @examples
+#' # nolint start
+#' \dontrun{
+#' isolate_example("contain side effects", {
+#' # Consider a simple plan that depends on the biglm package.
+#' # library(biglm)
+#' plan <- drake_plan(model = biglm(y ~ x, data = huge_dataset))
+#' # Even if you load the biglm package, drake still ignores
+#' # the biglm() function as a dependency. The function is missing
+#' # from the graph:
+#' # vis_drake_graph(plan)
+#' # And if you install an updated version of biglm with a revised
+#' # biglm() function, this will not cause drake::make(plan)
+#' # to rerun the model.
+#' # This is because biglm() is not in your environment.
+#' # ls()
+#' # biglm() exists in its own special package environment,
+#' # which drake does not scan.
+#' # ls("package:biglm")
+#' # To depend on biglm(), use expose_imports(biglm)
+#' # to bring the objects and functions in biglm into
+#' # your own (non-package) environment.
+#' # expose_imports(biglm)
+#' # Now, the biglm() function should be in your environment.
+#' # ls()
+#' # biglm() now appears in the graph.
+#' # vis_drake_graph(plan)
+#' # And subsequent make()s respond to changes to biglm()
+#' # and its dependencies.
+#' })
+#' }
+#' # nolint end
+expose_imports <- function(
+  package,
+  character_only = FALSE,
+  envir = parent.frame(),
+  jobs = 1
+) {
+  .Deprecated(
+    package = "drake",
+    msg = paste(
+      "expose_imports() is deprecated because of the edge cases it causes.",
+      "Instead, please supply getNamespace('yourPackage') to the envir",
+      "argument of make() and related functions. See",
+      "https://github.com/ropensci/drake/issues/1286 for details."
+    )
+  )
+  force(envir)
+  if (!character_only) {
+    package <- as.character(substitute(package))
+  }
+  expose_envir(from = getNamespace(package), to = envir, jobs = jobs)
+}
+
+expose_envir <- function(from, to, jobs, keep = names(from)) {
+  from <- as.list(from, all.names = TRUE)[keep]
+  from <- list2env(from, parent = globalenv())
+  lightly_parallelize(
+    X = ls(from, all.names = TRUE),
+    FUN = function(name) {
+      value <- get(name, envir = from)
+      if (typeof(value) == "closure") {
+        assign(
+          x = name,
+          envir = to,
+          value = `environment<-`(value, from)
+        )
+      } else {
+        assign(x = name, envir = to, value = value)
+      }
+    },
+    jobs = jobs
+  )
+  to
+}

R/expose_imports.R

@@ -1,93 +0,0 @@
-#' @title Expose package functions and objects for analysis with drake
-#' \lifecycle{stable}
-#' @description `drake` usually ignores the functions and objects
-#'   from packages. Use `expose_imports()` to bring the functions and
-#'   objects from a package into your environment. That way,
-#'   `drake` scans them for dependencies and watches for when
-#'   they change.
-#'   Thanks to [Jasper Clarkberg](https://github.com/dapperjapper)
-#'   for the implementation idea.
-#' @details
-#'   `expose_imports()` puts functions from your custom package
-#'   into a non-package environment. That means `@import` and `@importFrom`
-#'   functions will no longer be found if you are not using
-#'   namespaced calls (e.g. `pkg::fun()`). Please
-#'   read <https://github.com/ropensci/drake/issues/1286>
-#'   for details.
-#' @export
-#' @return The environment that the exposed imports are loaded into.
-#'   Defaults to your R workspace.
-#' @param package Name of the package, either a symbol or a string,
-#'   depending on `character_only`.
-#' @param character_only Logical, whether to interpret `package`
-#'   as a character string or a symbol (quoted vs unquoted).
-#' @param envir Environment to load the exposed package imports.
-#'   You will later pass this `envir` to [make()].
-#' @param jobs Number of parallel jobs for the parallel processing
-#'   of the imports.
-#' @examples
-#' # nolint start
-#' \dontrun{
-#' isolate_example("contain side effects", {
-#' # Consider a simple plan that depends on the biglm package.
-#' # library(biglm)
-#' plan <- drake_plan(model = biglm(y ~ x, data = huge_dataset))
-#' # Even if you load the biglm package, drake still ignores
-#' # the biglm() function as a dependency. The function is missing
-#' # from the graph:
-#' # vis_drake_graph(plan)
-#' # And if you install an updated version of biglm with a revised
-#' # biglm() function, this will not cause drake::make(plan)
-#' # to rerun the model.
-#' # This is because biglm() is not in your environment.
-#' # ls()
-#' # biglm() exists in its own special package environment,
-#' # which drake does not scan.
-#' # ls("package:biglm")
-#' # To depend on biglm(), use expose_imports(biglm)
-#' # to bring the objects and functions in biglm into
-#' # your own (non-package) environment.
-#' # expose_imports(biglm)
-#' # Now, the biglm() function should be in your environment.
-#' # ls()
-#' # biglm() now appears in the graph.
-#' # vis_drake_graph(plan)
-#' # And subsequent make()s respond to changes to biglm()
-#' # and its dependencies.
-#' })
-#' }
-#' # nolint end
-expose_imports <- function(
-  package,
-  character_only = FALSE,
-  envir = parent.frame(),
-  jobs = 1
-) {
-  force(envir)
-  if (!character_only) {
-    package <- as.character(substitute(package))
-  }
-  expose_envir(from = getNamespace(package), to = envir, jobs = jobs)
-}
-
-expose_envir <- function(from, to, jobs, keep = names(from)) {
-  from <- as.list(from, all.names = TRUE)[keep]
-  from <- list2env(from, parent = globalenv())
-  lightly_parallelize(
-    X = ls(from, all.names = TRUE),
-    FUN = function(name) {
-      value <- get(name, envir = from)
-      if (typeof(value) == "closure") {
-        assign(
-          x = name,
-          envir = to,
-          value = `environment<-`(value, from)
-        )
-      } else {
-        assign(x = name, envir = to, value = value)
-      }
-    },
-    jobs = jobs
-  )
-  to
-}

_pkgdown.yml

@@ -155,6 +155,3 @@ reference:
   - '`drake_done`'
   - '`drake_failed`'
   - '`drake_cancelled`'
-- title: R packages as drake workflows
-  contents:
-  - '`expose_imports`'

tests/testthat/test-4-imports.R

@@ -92,37 +92,6 @@ test_with_dir("add a new import", {
   expect_equal(readd(a, cache = cache), 5L)
 })
 
-test_with_dir("expose_imports() works", {
-  skip_on_cran() # CRAN gets whitelist tests only (check time limits).
-  scenario <- get_testing_scenario()
-  envir <- eval(parse(text = scenario$envir))
-  evalq(
-    f <- function(x) {
-      g(x)
-    },
-    envir = envir
-  )
-  evalq(
-    g <- function(x) {
-      digest(x)
-    },
-    envir = envir
-  )
-  plan <- drake_plan(
-    x = f(1),
-    y = digest::digest(x)
-  ) # double-scoped functions stop the nesting.
-  config <- drake_config(plan, envir = envir)
-  n_nodes <- length(igraph::V(config$graph)$name)
-  expect_true(n_nodes < 10)
-  envir <- expose_imports(digest, envir = envir)
-  config <- drake_config(plan, envir = envir)
-  n_nodes_new <- length(igraph::V(config$graph)$name)
-  expect_true(n_nodes_new > n_nodes)
-  make_impl(config = config)
-  expect_is(readd(x), "character")
-})
-
 # Target/import conflicts are unpredictable. A warning should
 # be enough.
 test_with_dir("target conflicts with current import or another target", {

tests/testthat/test-7-deprecate.R

@@ -1543,3 +1543,37 @@ test_with_dir("progress(), running(), and failed()", {
   expect_warning(run <- running(), regexp = "deprecated")
   expect_warning(fld <- failed(), regexp = "deprecated")
 })
+
+test_with_dir("expose_imports() works", {
+  skip_on_cran() # CRAN gets whitelist tests only (check time limits).
+  scenario <- get_testing_scenario()
+  envir <- eval(parse(text = scenario$envir))
+  evalq(
+    f <- function(x) {
+      g(x)
+    },
+    envir = envir
+  )
+  evalq(
+    g <- function(x) {
+      digest(x)
+    },
+    envir = envir
+  )
+  plan <- drake_plan(
+    x = f(1),
+    y = digest::digest(x)
+  ) # double-scoped functions stop the nesting.
+  config <- drake_config(plan, envir = envir)
+  n_nodes <- length(igraph::V(config$graph)$name)
+  expect_true(n_nodes < 10)
+  envir <- expect_warning(
+    expose_imports(digest, envir = envir),
+    message = "deprecated"
+  )
+  config <- drake_config(plan, envir = envir)
+  n_nodes_new <- length(igraph::V(config$graph)$name)
+  expect_true(n_nodes_new > n_nodes)
+  make_impl(config = config)
+  expect_is(readd(x), "character")
+})