Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

CodeDepends::getInputs() fails very strangely if filter() is part of the expression #268

Closed
krlmlr opened this issue Feb 20, 2018 · 19 comments
Closed

CodeDepends::getInputs() fails very strangely if filter() is part of the expression #268

krlmlr opened this issue Feb 20, 2018 · 19 comments

Comments

@krlmlr
Copy link
Collaborator

krlmlr commented Feb 20, 2018

Only a drake problem because it's now using CodeDepends. @gmbecker: Any idea what's wrong here?

CodeDepends::getInputs(quote(filter(x)))
#> Error in collector$vars: object of type 'closure' is not subsettable

Other function names seem to work fine. The x is necessary to trigger the problem.

Created on 2018-02-20 by the reprex package (v0.2.0).
@wlandau
Copy link
Member

wlandau commented Feb 20, 2018 •
edited
Loading

A tiny project of mine had this problem too, but then I restarted the R session and it went away. I am struggling to reproduce it.

> sessionInfo()
R version 3.4.0 (2017-04-21)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Red Hat Enterprise Linux Server release 6.9 (Santiago)

Matrix products: default
BLAS: /[censored]/R-3.4.0/lib64/R/lib/libRblas.so
LAPACK: /[censored]/R-3.4.0/lib64/R/lib/libRlapack.so

locale:
 [1] LC_CTYPE=en_US       LC_NUMERIC=C         LC_TIME=en_US
 [4] LC_COLLATE=en_US     LC_MONETARY=en_US    LC_MESSAGES=en_US
 [7] LC_PAPER=en_US       LC_NAME=C            LC_ADDRESS=C
[10] LC_TELEPHONE=C       LC_MEASUREMENT=en_US LC_IDENTIFICATION=C

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base

other attached packages:
[1] usethis_1.2.0.9000 testthat_2.0.0     devtools_1.13.4

loaded via a namespace (and not attached):
 [1] CodeDepends_0.5-5.2 compiler_3.4.0      backports_1.1.2
 [4] magrittr_1.5        R6_2.2.2            rprojroot_1.3-2
 [7] withr_2.1.1.9000    memoise_1.1.0       codetools_0.2-15
[10] digest_0.6.15       rlang_0.2.0         XML_3.98-1.10

@krlmlr
Copy link
Collaborator Author

krlmlr commented Feb 20, 2018

It fails reproducibly on my machine, Ubuntu 17.10, R 3.4.3.

@gmbecker
Copy link

@krlmlr what version of CodeDepends do you have? I'm unable to recreate that with the latest github version, in either the "dplyr was loaded" or "dplyr was not laoded" cases (which have radically different semantics, as you can see below.

> library(CodeDepends)
> getInputs(quote(filter(x)))
An object of class "ScriptNodeInfo"
Slot "files":
character(0)

Slot "strings":
character(0)

Slot "libraries":
character(0)

Slot "inputs":
[1] "x"

Slot "outputs":
character(0)

Slot "updates":
character(0)

Slot "functions":
filter 
    NA 

Slot "removes":
character(0)

Slot "nsevalVars":
character(0)

Slot "sideEffects":
character(0)

Slot "code":
filter(x)

> thing = readScript(txt = "library(dplyr); filter(df, x)")
> getInputs(thing)
An object of class "ScriptInfo"
[[1]]
An object of class "ScriptNodeInfo"
Slot "files":
character(0)

Slot "strings":
character(0)

Slot "libraries":
[1] "dplyr"

Slot "inputs":
character(0)

Slot "outputs":
character(0)

Slot "updates":
character(0)

Slot "functions":
named logical(0)

Slot "removes":
character(0)

Slot "nsevalVars":
character(0)

Slot "sideEffects":
character(0)

Slot "code":
library(dplyr)


[[2]]
An object of class "ScriptNodeInfo"
Slot "files":
character(0)

Slot "strings":
character(0)

Slot "libraries":
character(0)

Slot "inputs":
[1] "df"

Slot "outputs":
character(0)

Slot "updates":
character(0)

Slot "functions":
filter 
 FALSE 

Slot "removes":
character(0)

**Slot "nsevalVars":
[1] "x"**

Slot "sideEffects":
character(0)

Slot "code":
filter(df, x)


> getInputs(quote(filter(df, x)))
An object of class "ScriptNodeInfo"
Slot "files":
character(0)

Slot "strings":
character(0)

Slot "libraries":
character(0)

**Slot "inputs":
[1] "df" "x"** 

Slot "outputs":
character(0)

Slot "updates":
character(0)

Slot "functions":
filter 
    NA 

Slot "removes":
character(0)

Slot "nsevalVars":
character(0)

Slot "sideEffects":
character(0)

Slot "code":
filter(df, x)

@tiernanmartin
Copy link
Contributor

Something is definitely amiss with filter():

library(drake)
library(tidyverse) 

my_filter <- function(x) {
  filter(x, cyl == 4)
}

failing_plan <- drake_plan(
  new_mtcars = my_filter(mtcars)
)

make(failing_plan) # fails
## cache C:\Users\UrbanDesigner\AppData\Local\Temp\Rtmp2XZhXv\.drake
## connect 2 imports: my_filter, failing_plan
## Error in collector$vars: object of type 'closure' is not subsettable

But dplyr::filter() works:

library(drake)
library(tidyverse) 

my_other_filter <- function(x) {
  dplyr::filter(x, cyl == 4)
}

working_plan <- drake_plan(
  new_mtcars = my_other_filter(mtcars)
)


make(working_plan) # works
## cache C:\Users\UrbanDesigner\AppData\Local\Temp\Rtmpq6W0mq\.drake
## connect 2 imports: my_other_filter, working_plan
## connect 1 target: new_mtcars
## check 2 items: dplyr::filter, mtcars
## check 1 item: my_other_filter
## check 1 item: new_mtcars
## target new_mtcars
Session info 
devtools::session_info()
## Session info -------------------------------------------------------------
##  setting  value                       
##  version  R version 3.4.0 (2017-04-21)
##  system   x86_64, mingw32             
##  ui       RTerm                       
##  language (EN)                        
##  collate  English_United States.1252  
##  tz       America/Los_Angeles         
##  date     2018-02-20
## Packages -----------------------------------------------------------------
##  package      * version    date       source                            
##  assertthat     0.2.0      2017-04-11 CRAN (R 3.4.2)                    
##  backports      1.1.0      2017-05-22 CRAN (R 3.4.0)                    
##  base         * 3.4.0      2017-04-21 local                             
##  bindr          0.1        2016-11-13 CRAN (R 3.4.2)                    
##  bindrcpp       0.2        2017-06-17 CRAN (R 3.4.2)                    
##  broom          0.4.3      2017-11-20 CRAN (R 3.4.3)                    
##  cellranger     1.1.0      2016-07-27 CRAN (R 3.4.2)                    
##  cli            1.0.0      2017-11-05 CRAN (R 3.4.2)                    
##  CodeDepends    0.5-3      2017-05-29 CRAN (R 3.4.3)                    
##  codetools      0.2-15     2016-10-05 CRAN (R 3.4.0)                    
##  colorspace     1.3-2      2016-12-14 CRAN (R 3.4.2)                    
##  compiler       3.4.0      2017-04-21 local                             
##  crayon         1.3.4      2018-02-12 Github (r-lib/crayon@95b3eae)     
##  datasets     * 3.4.0      2017-04-21 local                             
##  devtools       1.13.2     2017-06-02 CRAN (R 3.4.0)                    
##  digest         0.6.15     2018-01-28 CRAN (R 3.4.3)                    
##  dplyr        * 0.7.4      2017-09-28 CRAN (R 3.4.2)                    
##  drake        * 5.0.1.9002 2018-02-21 Github (ropensci/drake@cfac8fd)   
##  evaluate       0.10.1     2017-06-24 CRAN (R 3.4.3)                    
##  forcats      * 0.2.0      2017-01-23 CRAN (R 3.4.3)                    
##  foreign        0.8-67     2016-09-13 CRAN (R 3.4.0)                    
##  formatR        1.5        2017-04-25 CRAN (R 3.4.0)                    
##  future         1.7.0      2018-02-11 CRAN (R 3.4.3)                    
##  future.apply   0.1.0      2018-01-15 CRAN (R 3.4.3)                    
##  ggplot2      * 2.2.1.9000 2017-12-02 Github (tidyverse/ggplot2@7b5c185)
##  globals        0.11.0     2018-01-10 CRAN (R 3.4.3)                    
##  glue           1.2.0.9000 2018-01-13 Github (tidyverse/glue@1592ee1)   
##  graphics     * 3.4.0      2017-04-21 local                             
##  grDevices    * 3.4.0      2017-04-21 local                             
##  grid           3.4.0      2017-04-21 local                             
##  gtable         0.2.0      2016-02-26 CRAN (R 3.4.2)                    
##  haven          1.1.0      2017-07-09 CRAN (R 3.4.2)                    
##  hms            0.4.0      2017-11-23 CRAN (R 3.4.3)                    
##  htmltools      0.3.6      2017-04-28 CRAN (R 3.4.0)                    
##  htmlwidgets    0.9        2017-07-10 CRAN (R 3.4.2)                    
##  httr           1.3.1      2017-08-20 CRAN (R 3.4.2)                    
##  igraph         1.1.2      2017-07-21 CRAN (R 3.4.3)                    
##  jsonlite       1.5        2017-06-01 CRAN (R 3.4.0)                    
##  knitr          1.19       2018-01-29 CRAN (R 3.4.3)                    
##  lattice        0.20-35    2017-03-25 CRAN (R 3.4.0)                    
##  lazyeval       0.2.1      2017-10-29 CRAN (R 3.4.2)                    
##  listenv        0.6.0      2015-12-28 CRAN (R 3.4.3)                    
##  lubridate      1.7.2      2018-02-06 CRAN (R 3.4.3)                    
##  magrittr       1.5        2014-11-22 CRAN (R 3.4.0)                    
##  memoise        1.1.0      2017-04-21 CRAN (R 3.4.0)                    
##  methods      * 3.4.0      2017-04-21 local                             
##  mnormt         1.5-5      2016-10-15 CRAN (R 3.4.1)                    
##  modelr         0.1.1      2017-07-24 CRAN (R 3.4.2)                    
##  munsell        0.4.3      2016-02-13 CRAN (R 3.4.2)                    
##  nlme           3.1-131    2017-02-06 CRAN (R 3.4.0)                    
##  parallel       3.4.0      2017-04-21 local                             
##  pillar         1.1.0.9000 2018-02-12 Github (r-lib/pillar@595d1ac)     
##  pkgconfig      2.0.1      2017-03-21 CRAN (R 3.4.2)                    
##  plyr           1.8.4      2016-06-08 CRAN (R 3.4.2)                    
##  psych          1.7.8      2017-09-09 CRAN (R 3.4.2)                    
##  purrr        * 0.2.4.9000 2017-12-05 Github (tidyverse/purrr@62b135a)  
##  R.methodsS3    1.7.1      2016-02-16 CRAN (R 3.4.1)                    
##  R.oo           1.21.0     2016-11-01 CRAN (R 3.4.1)                    
##  R.utils        2.6.0      2017-11-05 CRAN (R 3.4.3)                    
##  R6             2.2.2      2017-06-17 CRAN (R 3.4.0)                    
##  Rcpp           0.12.15    2018-01-20 CRAN (R 3.4.3)                    
##  readr        * 1.1.1      2017-05-16 CRAN (R 3.4.2)                    
##  readxl         1.0.0      2017-04-18 CRAN (R 3.4.2)                    
##  reshape2       1.4.2      2016-10-22 CRAN (R 3.4.2)                    
##  rlang          0.1.6      2017-12-21 CRAN (R 3.4.3)                    
##  rmarkdown      1.8        2017-11-17 CRAN (R 3.4.2)                    
##  rprojroot      1.3-2      2018-01-03 CRAN (R 3.4.3)                    
##  rvest          0.3.2      2016-06-17 CRAN (R 3.4.2)                    
##  scales         0.5.0.9000 2017-12-02 Github (hadley/scales@d767915)    
##  stats        * 3.4.0      2017-04-21 local                             
##  storr          1.1.3      2017-12-15 CRAN (R 3.4.3)                    
##  stringi        1.1.6      2017-11-17 CRAN (R 3.4.2)                    
##  stringr      * 1.3.0      2018-02-19 CRAN (R 3.4.3)                    
##  testthat       2.0.0      2017-12-13 CRAN (R 3.4.3)                    
##  tibble       * 1.4.2      2018-02-12 Github (tidyverse/tibble@80ac1a3) 
##  tidyr        * 0.7.2.9000 2018-01-13 Github (tidyverse/tidyr@74bd48f)  
##  tidyverse    * 1.2.1      2017-11-14 CRAN (R 3.4.3)                    
##  tools          3.4.0      2017-04-21 local                             
##  utils        * 3.4.0      2017-04-21 local                             
##  visNetwork     2.0.3      2018-01-09 CRAN (R 3.4.3)                    
##  withr          2.1.1.9000 2018-02-21 Github (jimhester/withr@4dfd12a)  
##  XML            3.98-1.9   2017-06-19 CRAN (R 3.4.1)                    
##  xml2           1.1.1      2017-01-24 CRAN (R 3.4.2)                    
##  yaml           2.1.14     2016-11-12 CRAN (R 3.4.0)

@wlandau
Copy link
Member

wlandau commented Feb 21, 2018

Could CodeDepends be failing on a filter() from another package?

@gmbecker
Copy link

gmbecker commented Feb 21, 2018 via email

@gmbecker
Copy link

gmbecker commented Feb 21, 2018 via email

@krlmlr
Copy link
Collaborator Author

krlmlr commented Feb 21, 2018

Works for me with the GitHub version. I wasn't aware this package is on GitHub, I looked at its DESCRIPTION only.

@krlmlr krlmlr closed this as completed Feb 21, 2018
@gmbecker
Copy link

For the record I'll work in getting a new version of CodeDepends onto CRAN this week which will have this fix in it.

@AlexAxthelm AlexAxthelm mentioned this issue Mar 13, 2018
Closed
@wlandau wlandau mentioned this issue Mar 25, 2018
Closed
@ruaridhw ruaridhw mentioned this issue Apr 6, 2018
Closed
@lorenzwalthert
Copy link
Contributor

I just had the same problem. After updating CodeDepends, it worked. Maybe a minimal version dependency should be declared for CodeDepends?

@wlandau
Copy link
Member

wlandau commented May 21, 2018

What about something a less strict, maybe suggesting a minimal version in this warning message? I hesitate to force a certain version of a package in order to patch an edge case.

@lorenzwalthert
Copy link
Contributor

lorenzwalthert commented May 21, 2018 •
edited
Loading

I wonder though whether it had to do with filter() in particular or whether it is a problem for all name space collisions. I think one also has to weight in the complexity that is added in the drake project when considering adding informative warning messages for these kinds of issues. But I mean it's up to you what to do. Just wanted to make sure this is somehow addressed.

@krlmlr
Copy link
Collaborator Author

krlmlr commented May 21, 2018

@gmbecker: Do you still plan to update CodeDepends on CRAN?

@wlandau: Requiring a minimum package version seems like a viable solution to fix a problem. Packages tend to get better, not worse. Would you be open to a "diagnosis" facility that compares installed vs. suggested package versions?

@wlandau
Copy link
Member

wlandau commented May 21, 2018 •
edited
Loading

You know what, I think you guys are right. I am seeing minimal version specs all across the tidyverse, so this is not likely to surprise people. @lorenzwalthert, what version of CodeDepends worked for you?

@lorenzwalthert
Copy link
Contributor

lorenzwalthert commented May 21, 2018 •
edited
Loading

I am actually not sure if we are talking about the same thing. Unfortunately, I can't reproduce what I had before updating, but I think I get a different error now if I don't use the package mask.

I.e. this works:

library("drake")
library("magrittr")
drake_plan(
  x = dplyr::filter(mtcars, cyl == 4)
) %>%
  make()
#> All targets are already up to date.
table(readd(x)$cyl)
#> 
#>  4 
#> 11

And this gives an error

library("drake")
library("magrittr")
library("dplyr")
#> 
#> Attaching package: 'dplyr'
#> The following objects are masked from 'package:drake':
#> 
#>     contains, ends_with, everything, matches, num_range, one_of,
#>     starts_with
#> The following objects are masked from 'package:stats':
#> 
#>     filter, lag
#> The following objects are masked from 'package:base':
#> 
#>     intersect, setdiff, setequal, union
drake_plan(
  x = filter(mtcars, cyl == 4)
) %>%
  make()
#> Warning: could not resolve implicit dependencies of
#> code:structure(expression(filter(mtcars, cyl == 4)), srcfile =
#> <environment>, wholeSrcref = structure(c(1L, 0L, 2L, 0L, 0L, 0L, 1L, 2L),
#> srcfile = <environment>, class = "srcref"))
#>  ...

I also run the code from #268 (comment), and I get not the same error message as @tiernanmartin got, but the same as in my second example.

I think it's good practice anyways to use the mask for the name space collision so I don't object to the behaviour demonstrated, although the error message could still be more informative.

So maybe the minimal version dependency does not make sense since it is not really a huge benefit we'd get with it, just a different error message.
I also tried the GitHub version, but installation fails due a problem in the NAMESPACE file so I can't make a comparison without some work.
Sorry for the confusion -.-

I tried opening an issue but it's not possible with forks I just realized. @tiernanmartin maybe it is worth fixing the problem. I get:

Using github PAT from envvar GITHUB_PAT
Downloading GitHub repo gmbecker/CodeDepends@master
* installing *source* package ‘CodeDepends’ ...
** R
Error in parse(outFile) : 
  /private/var/folders/8l/fhmv3yj12kncddcjqwc19tkr0000gr/T/RtmpDi5lyP/remotes6c9666522534/gmbecker-CodeDepends-d085a4f/R/codeDe:114:18: unexpected '}'
113:                    inputs <<- unique(c(inputs, 
114:                  }
                      ^
ERROR: unable to collate and parse R files for package ‘CodeDepends’
* removing ‘/Library/Frameworks/R.framework/Versions/3.4/Resources/library/CodeDepends’
* restoring previous ‘/Library/Frameworks/R.framework/Versions/3.4/Resources/library/CodeDepends’
Warning messages:
1: In utils::install.packages(pkgs = pkgs, lib = lib, repos = myrepos,  :
  installation of package ‘/var/folders/8l/fhmv3yj12kncddcjqwc19tkr0000gr/T//RtmpDi5lyP/remotes6c9666522534/gmbecker-CodeDepends-d085a4f’ had non-zero exit status
2: In utils::install.packages(pkgs = pkgs, lib = lib, repos = myrepos,  :
  installation of package ‘/var/folders/8l/fhmv3yj12kncddcjqwc19tkr0000gr/T//RtmpDi5lyP/remotes6c9666522534/gmbecker-CodeDepends-d085a4f’ had non-zero exit status

Probably another devtools::document() helps, or at least tells you where the problem is.

@wlandau
Copy link
Member

wlandau commented May 24, 2018

Well shucks, I thought the filter() issue in CodeDepends was already taken care of.

I do agree that drake should output a more informative warning. The trouble is that the part of drake that does the code analysis is not aware of the function name or target that the code corresponds to.

@gmbecker
Copy link

@lorenzwalthert The canonical sources for CodeDepends are at duncantl/CodeDepends. I am the maintainer but Duncan is the original author. doing install_github from that repo works correctly without error.

@krlmlr I do plan to update CodeDepends on CRAN. I need to figure out the masking warnings by tightening up the imports, but once I do that the new version should be on CRAN shortly after that. Apologies for the delay on that.

My suspicion is that these folks are running into problems because they are using the CRAN version. I'll try to get an updated version up soon, though I'm a bit buried under the upcoming breaking changes for ggplot2 on another project right this second :-/

@lorenzwalthert
Copy link
Contributor

Thanks @gmbecker I tried to install the fork, can't recall exactly why. But I also can't install the upstream.

> remotes::install_github("duncantl/CodeDepends")
Using github PAT from envvar GITHUB_PAT
Downloading GitHub repo duncantl/CodeDepends@master
Skipping 1 packages not available: graph
ERROR: dependency ‘graph’ is not available for package ‘CodeDepends’
* removing ‘/Library/Frameworks/R.framework/Versions/3.5/Resources/library/CodeDepends’
Warning message:
In i.p(...) :
  installation of package ‘/var/folders/8l/fhmv3yj12kncddcjqwc19tkr0000gr/T//RtmpECpFzn/remotes18b01fdcfbad/duncantl-CodeDepends-7b398fe’ had non-zero exit status

@gmbecker
Copy link

gmbecker commented May 24, 2018 via email

This was referenced Aug 16, 2018
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@gmbecker @wlandau @krlmlr @tiernanmartin @lorenzwalthert