Merge pull request #321 from ropensci/i320

Fix #320: ignore the `magrittr` dot (`.`)

NEWS.md

@@ -1,5 +1,6 @@
 # Version 5.1.0
 
+- Forcibly exclude the dot (`.`) from being a dependency of any target or import. This enforces more consistent behavior in the face of the current static code analysis funcionality, which sometimes detects `.` and sometimes does not.
 - Use `ignore()` to optionally ignore pieces of workflow plan commands and/or imported functions. Use `ignore(some_code)` to
     1. Force `drake` to not track dependencies in `some_code`, and
     2. Ignore any changes in `some_code` when it comes to deciding which target are out of date.

R/dependencies.R

@@ -353,14 +353,14 @@ find_globals <- function(expr){
   }
   # Warning: In collector$results(reset = reset) :
   #  partial argument match of 'reset' to 'resetState'
-
   suppressWarnings(inputs <- CodeDepends::getInputs(expr))
   base::union(
     inputs@inputs,
     names(inputs@functions)
   ) %>%
     setdiff(y = c(formals, drake_fn_patterns)) %>%
-    Filter(f = is_parsable)
+    Filter(f = is_parsable) %>%
+    setdiff(y = ".") # Exclude the magrittr dot
 }
 
 analyze_loadd <- function(expr){

tests/testthat/test-dependencies.R

@@ -6,6 +6,17 @@ test_with_dir("unparsable commands are handled correctly", {
   expect_error(deps(x))
 })
 
+test_with_dir("magrittr dot is ignored", {
+  expect_equal(
+    sort(deps("sqrt(x + y + .)")),
+    sort(c("sqrt", "x", "y"))
+  )
+  expect_equal(
+    sort(deps("dplyr::filter(complete.cases(.))")),
+    sort(c("complete.cases", "dplyr::filter"))
+  )
+})
+
 test_with_dir("file_out() and knitr_in(): commands vs imports", {
   cmd <- "file_in(\"x\"); file_out(\"y\"); knitr_in(\"report.Rmd\")"
   f <- function(){

vignettes/caution.Rmd

@@ -196,6 +196,15 @@ You can call `make(..., timeout = 10)` to time out all each target after 10 seco
 
 # Dependencies
 
+## The `magrittr` dot (`.`) is always ignored.
+
+It is common practice to use a literal dot (`.`) to carry an object through a [`magrittr`](https://github.com/tidyverse/magrittr) pipeline. Due to some tricky  limitations in static code analysis, `drake` never treats the dot (`.`) as a dependency, even if you use it as an ordinary variable outside of a [`tidyverse`](https://www.tidyverse.org/) context.
+
+```{r depsdot}
+deps("sqrt(x + y + .)")
+deps("dplyr::filter(complete.cases(.))")
+```
+
 ## Triggers and skipped imports
 
 With alternate [triggers](https://github.com/ropensci/drake/blob/master/vignettes/debug.Rmd#test-with-triggers) and the [option to skip imports](https://github.com/ropensci/drake/blob/master/vignettes/debug.Rmd#skipping-imports), you can sacrifice reproducibility to gain speed. However, these options can throw the dependency network out of sync. You should only use them for testing and debugging.