benmarwick / rrtools

rrtools: Tools for Writing Reproducible Research in R
Other
670 stars 85 forks source link

`paper.Rmd` assumes rocker image has already installed `devtools, git2r, here` #89

Closed ntrlshrp closed 4 years ago

ntrlshrp commented 5 years ago

Some recent versions of rocker/verse, e.g., 3.5.3, may not have all packages explicitly used in paper.Rmd default (i.e., devtools, git2r, here), all of which are required to knit paper.Rmd. That is, Dockerfile with rocker/verse:3.5.3 fails with Quitting from lines 89-91 (paper.Rmd) Error in loadNamespace(name) : there is no package called 'here'.

benmarwick commented 5 years ago

Thanks for noting this, could you give a more specific report of the conditions under which you observe this problem?

I'm not sure that adding install.packages in the Rmd, as you do in #90, is the best way to fix this. If you do that, then each time the Rmd is knit those packages will be installed, which in many cases in unnecessary and wasteful.

If you are noticing this error on travis, then probably we should put the install.packages function in the dockerfile. That's why we need a bit more information from you before we can decide what is the best fix.

As a point of etiquette, could you please wait for some discussion of your issue before proposing a pull request? We are all volunteers here, and a thoughtful discussion before writing any code will save everyone's time and ensure a productive and respectful exchange of ideas.

ntrlshrp commented 5 years ago

I can absolutely do pull requests after discussion! :) Here's a better bug report:

Expected behavior: Docker can build the default paper.Rmd on recent versions of rocker/verse

Actual behavior: Using rocker/verse:3.5.3, Dockerfile fails with Quitting from lines 89-91 (paper.Rmd) Error in loadNamespace(name) : there is no package called 'here'

Steps to reproduce:

  1. In R, devtools::install_github("benmarwick/rrtools")
  2. rrtools::create_compendium("rrtoolsTEST")
  3. Switch to new project/package rrtoolsTEST
  4. rrtools::use_dockerfile()
  5. Change rocker/verse:### to rocker/verse:3.5.3 in rrtoolsTEST/Dockerfile line 1
  6. Switch to Terminal where current directory is rrtoolsTEST/
  7. docker build --rm -t rrtoolstest:1.0 . (you may need to prepend sudo)
    • Note: this is not Travis
Quitting from lines 89-91 (paper.Rmd) 
Error in loadNamespace(name) : there is no package called 'here'
Calls: <Anonymous> ... tryCatch -> tryCatchList -> tryCatchOne -> <Anonymous>

Execution halted
The command '/bin/sh -c . /etc/environment   && sudo apt-get update   && sudo apt-get install libudunits2-dev -y   && R -e "devtools::install('/<REPO>', dep=TRUE)"   && R -e "rmarkdown::render('/<REPO>/analysis/paper/paper.Rmd')"' returned a non-zero code: 1`

Notes:

Potential solutions (very open to others' thoughts which likely see a better way to do this):

if(!require(somepackage)){
    install.packages("somepackage")
    library(somepackage)
}
nevrome commented 5 years ago

I can reproduce the issue.

@benmarwick Do we really need this Colophon section in the paper.Rmd?

##### pagebreak

### Colophon

This report was generated on `r Sys.time()` using the following computational environment and dependencies: 

``{r colophon, cache = FALSE}
# which R packages and versions?
devtools::session_info()
``

The current Git commit details are:

``{r}
# what commit is this file at? 
git2r::repository(here::here())
``

Removing it solves the problem. And in my opinion it's better not to clutter the paper repo images with these heavy dependencies just for some default text. Or am I missing something?

Otherwise I vote for the first solution suggested by @ntrlshrp:

Add any packages used by default paper.Rmd (e.g., devtools, git2r, here) to the Imports field of the DESCRIPTION file for rrtoolsTEST (currently, Imports: bookdown)

This can be added here, I guess:

https://github.com/benmarwick/rrtools/blob/4fdd3983941b56c0755fcd473f3ec599f5b73480/R/core_use_analysis.R#L20

ntrlshrp commented 5 years ago

Picking this back up:

This thread hasn't received much action, so I wonder if others like any of the following for a PR:

  1. Removing [the Colophon]

  2. Add any packages used by default paper.Rmd (e.g., devtools, git2r, here) to the Imports field of the DESCRIPTION file for rrtoolsTEST (currently, Imports: bookdown)

  3. Other

  4. No PR