JoSS review (reviewer #2)

[cole-brokamp]

Hello, opening up an issue separately from @vsoch's here for some of my more R specific questions/comments. I'll try not to duplicate anything that she has already brought up.

Installation

I also ran into issue installing the package. remotes::install_github() returned an error:

Error: HTTP error 422.
  No commit found for SHA: gaborcsardi/gh

but using pak::pkg_install() worked just fine. We need to either determine the root cause of this install failure or provide user with alternative install instructions. I wonder if its because pak doesn't install suggested dependencies unless explicitly directed to?

Statement of Need

The first sentence of the readme does state what the software does, but it might be helpful to also identify targeted users... maybe R users who are unfamiliar with creating Dockerfiles and containers?

Functionality Documentation

Trying to run the example in the README seems to work for generating the dockerfile object, but printing it to the console does not result in what is in the README. Instead, I get what appears to be a standard printing of the S6 object's slots:

An object of class "Dockerfile"
Slot "image":
An object of class "From"
Slot "image":
[1] "rocker/r-ver"

Slot "postfix":
An object of class "Tag"
[1] "3.5.3"


Slot "maintainer":
An object of class "Label"
Slot "data":
$maintainer
[1] "cole"


Slot "multi_line":
[1] FALSE


Slot "instructions":
[[1]]
An object of class "Run_shell"
Slot "commands":
[1] "export DEBIAN_FRONTEND=noninteractive; apt-get -y update"                                            
[2] "apt-get install -y git-core \\\n\tlibcurl4-openssl-dev \\\n\tlibssl-dev \\\n\tlibxml2-dev \\\n\tmake"


[[2]]
An object of class "Run"
Slot "exec":
[1] "install2.r"

Slot "params":
 [1] "assertthat"     "backports"      "broom"          "callr"          "cellranger"     "cli"            "codetools"      "colorspace"     "crayon"        
[10] "curl"           "desc"           "digest"         "dplyr"          "forcats"        "formatR"        "fs"             "futile.logger"  "futile.options"
[19] "generics"       "ggplot2"        "glue"           "gtable"         "haven"          "hms"            "htmltools"      "httpuv"         "httr"          
[28] "jsonlite"       "lambda.r"       "later"          "lattice"        "lazyeval"       "lintr"          "lubridate"      "magrittr"       "mime"          
[37] "miniUI"         "modelr"         "munsell"        "nlme"           "pillar"         "pkgbuild"       "pkgconfig"      "plyr"           "prettyunits"   
[46] "processx"       "promises"       "ps"             "purrr"          "R6"             "Rcpp"           "readr"          "readxl"         "remotes"       
[55] "rex"            "rlang"          "rprojroot"      "rstudioapi"     "rvest"          "scales"         "semver"         "shiny"          "shinyFiles"    
[64] "stevedore"      "stringi"        "stringr"        "tibble"         "tidyr"          "tidyselect"     "tidyverse"      "vctrs"          "versions"      
[73] "withr"          "xml2"           "xtable"         "zeallot"       


[[3]]
An object of class "Run"
Slot "exec":
[1] "installGithub.r"

Slot "params":
[1] "cole-brokamp/CB@7ea91855c70c9586ee9546ecbdb10b044f3ef100"    "hadley/memoise@1650ad7f9c27d2e0a932e50b16eedb574e8050df"    
[3] "jalvesaq/colorout@cc5fbfa1f165bd4762588b463157a2688b116c5f"  "jimhester/highlite@767b122ef47a60a01e1707e4093cf3635a99c86b"
[5] "jimhester/lookup@e1e94d334e55660709d995f651bc746f7b7be4f1"   "r-lib/pak@7a69eaa18b7a250d044e9c6e948ff8930823a314"         


[[4]]
An object of class "Workdir"
Slot "path":
[1] "/payload/"



Slot "entrypoint":
NULL

Slot "cmd":
An object of class "Cmd"
Slot "exec":
[1] "R"

Slot "params":
[1] NA

Slot "form":
[1] "exec"

Hopefully this isn't something wrong with my installation, but no matter the cause, the core functionality doesn't seem to proceed as advertised in this case.

Manuscript

• you may want to consider listing as a another related work
• in Related Work section: "namerly" -> "namely"

[cole-brokamp]

As suggested by @vsoch here: #156, it would be great to have instructions for using a docker container to try out the package without having to install R and the long list of dependencies. Specifically, adding a part of your vignette (https://o2r.info/containerit/articles/container.html) to the README would help new users try it out easier and quicker!

[nuest]

I've started a PR to track the changes after the first round of review: #160

Re. the printing of the object rather than the function: How did you install the package? There is a function that should print the Dockerfile and not the object, but maybe it was not properly registered in your installation with pak? I will investigate.

Re. users in the statement of need: extended the last sentence of the first paragraph to make that a bit more explicit: nuest@08e1504
Better?

[nuest]

I've tried installation with pak in a rocker/geospatial container, and had the same result when I did not load the package via library(..) but only used it via containerit::..:

daniel@nuest:~$ docker run --rm -it rocker/geospatial R
> install.packages("pak")
> pak::pkg_install(pkg = "o2r-project/containerit")
ℹ Checking for package metadata updates
✔ Downloaded metadata files, 1.6 MB in 6 files.                                 
✔ Updating metadata database                                                    
✔ Using cached package metadata                                                 
                                                                                
→ Will install 12 packages:
  o2r-project/containerit, automagic, futile.logger, futile.options, lambda.r,
  rjson, semver, shinyFiles, stevedore, versions, github::r-hub/sysreqs,
  debugme
  
→ Will not update 69 packages.

→ Will download 10 CRAN packages (2.96 MB).
→ Will download 2 packages with unknown size.

? Do you want to continue (Y/n) y
ℹ Building automagic 0.5.1                                                      
ℹ Building futile.options 1.0.1                                                 
ℹ Building lambda.r 1.2.3                                                       
ℹ Building rjson 0.2.20                                                         
ℹ Building semver 0.2.0                                                         
ℹ Building shinyFiles 0.7.3                                                     
ℹ Building stevedore 0.9.1                                                      
ℹ Building versions 0.3                                                         
✔ Built automagic 0.5.1 [3.1s]                                                  ..
ℹ Building debugme 1.1.0                                                        ..
✔ Built futile.options 1.0.1 [3.4s]                                             ..
✔ Installed automagic 0.5.1  [88ms]                                             ..
✔ Installed futile.options 1.0.1  [112ms]                                       ..
✔ Built versions 0.3 [3.6s]                                                     ..
✔ Installed versions 0.3  [64ms]                                                ..
✔ Built lambda.r 1.2.3 [5.6s]                                                   ..
✔ Installed lambda.r 1.2.3  [65ms]                                              ..
ℹ Building futile.logger 1.4.3                                                  ..
✔ Built rjson 0.2.20 [5.9s]                                                     ..
✔ Installed rjson 0.2.20  [86ms]                                                ..
✔ Built debugme 1.1.0 [3.5s]                                                    ..
✔ Installed debugme 1.1.0  [130ms]                                              ..
ℹ Building sysreqs 1.0.0.9000                                                   ..
✔ Built shinyFiles 0.7.3 [7.4s]                                                 ..
✔ Installed shinyFiles 0.7.3  [92ms]                                            ..
✔ Built futile.logger 1.4.3 [3.5s]                                              ..
✔ Built sysreqs 1.0.0.9000 [2.6s]                                               ..
✔ Installed futile.logger 1.4.3  [101ms]                                        ..
✔ Installed sysreqs 1.0.0.9000 (github::r-hub/sysreqs@3860f2b) [37ms]           ..
✔ Built stevedore 0.9.1 [10.8s]                                                 ..
✔ Installed stevedore 0.9.1  [47ms]                                             ..
✔ Built semver 0.2.0 [16.5s]                                                    ..
✔ Installed semver 0.2.0  [70ms]                                                
ℹ Building containerit 0.5.0                                                    
✔ Built containerit 0.5.0 [5.5s]                                                
✔ Installed containerit 0.5.0 (github::o2r-project/containerit@de6749f) [48ms]  
✔ 1 + 80 pkgs | kept 69, updated 0, new 12 | downloaded 12 (3.18 MB) [42s]      
> df <- containerit::dockerfile()
INFO [2019-08-16 07:57:41] Going online? TRUE  ... to retrieve system dependencies (sysreq-api)
INFO [2019-08-16 07:57:41] Trying to determine system requirements for the package(s) 'assertthat,backports,callr,crayon,curl,desc,digest,filelock,formatR,fs,futile.logger,futile.options,htmltools,httpuv,jsonlite,lambda.r,later,magrittr,mime,miniUI,pak,pillar,pkgconfig,processx,promises,ps,R6,Rcpp,rlang,rprojroot,semver,shiny,shinyFiles,stevedore,stringi,stringr,tibble,versions,xtable' from sysreqs online DB
INFO [2019-08-16 07:57:59] Adding CRAN packages: assertthat, backports, callr, crayon, curl, desc, digest, filelock, formatR, fs, futile.logger, futile.options, htmltools, httpuv, jsonlite, lambda.r, later, magrittr, mime, miniUI, pak, pillar, pkgconfig, processx, promises, ps, R6, Rcpp, rlang, rprojroot, semver, shiny, shinyFiles, stevedore, stringi, stringr, tibble, versions, xtable
INFO [2019-08-16 07:57:59] Created Dockerfile-Object based on sessionInfo
> print(df)
An object of class "Dockerfile"
Slot "image":
An object of class "From"
Slot "image":
[1] "rocker/r-ver"

Slot "postfix":
An object of class "Tag"
[1] "3.6.0"


Slot "maintainer":
An object of class "Label"
Slot "data":
$maintainer
[1] "root"


Slot "multi_line":
[1] FALSE


Slot "instructions":
[[1]]
An object of class "Run_shell"
Slot "commands":
[1] "export DEBIAN_FRONTEND=noninteractive; apt-get -y update"           
[2] "apt-get install -y libcurl4-openssl-dev \\\n\tlibssl-dev \\\n\tmake"


[[2]]
An object of class "Run"
Slot "exec":
[1] "install2.r"

Slot "params":
 [1] "assertthat"     "backports"      "callr"          "crayon"        
 [5] "curl"           "desc"           "digest"         "filelock"      
 [9] "formatR"        "fs"             "futile.logger"  "futile.options"
[13] "htmltools"      "httpuv"         "jsonlite"       "lambda.r"      
[17] "later"          "magrittr"       "mime"           "miniUI"        
[21] "pak"            "pillar"         "pkgconfig"      "processx"      
[25] "promises"       "ps"             "R6"             "Rcpp"          
[29] "rlang"          "rprojroot"      "semver"         "shiny"         
[33] "shinyFiles"     "stevedore"      "stringi"        "stringr"       
[37] "tibble"         "versions"       "xtable"        


[[3]]
An object of class "Workdir"
Slot "path":
[1] "/payload/"



Slot "entrypoint":
NULL

Slot "cmd":
An object of class "Cmd"
Slot "exec":
[1] "R"

Slot "params":
[1] NA

Slot "form":
[1] "exec"


> library("containerit")
> print(df)
FROM rocker/r-ver:3.6.0
LABEL maintainer="root"
RUN export DEBIAN_FRONTEND=noninteractive; apt-get -y update \
  && apt-get install -y libcurl4-openssl-dev \
	libssl-dev \
	make
RUN ["install2.r", "assertthat", "backports", "callr", "crayon", "curl", "desc", "digest", "filelock", "formatR", "fs", "futile.logger", "futile.options", "htmltools", "httpuv", "jsonlite", "lambda.r", "later", "magrittr", "mime", "miniUI", "pak", "pillar", "pkgconfig", "processx", "promises", "ps", "R6", "Rcpp", "rlang", "rprojroot", "semver", "shiny", "shinyFiles", "stevedore", "stringi", "stringr", "tibble", "versions", "xtable"]
WORKDIR /payload/
CMD ["R"]

Can you confirm that behaviour?

[cole-brokamp]

I think this is a moot point because my problems with remotes::install_github('o2r-project/containerit') disappeared when using a fresh R version in docker (with either MRAN or CRAN used to support the install). So I'm going to chalk this up to a problem with one of the dependencies in my local library. Things proceed as expected now.

[cole-brokamp]

The RStudio addins are so very cool!!

You can take or leave my suggestions below about the manuscript, but everything for the review portion looks complete. Thanks!

Manuscript

* you may want to consider listing ![renv](https://github.com/rstudio/renv) as a another related work

* in Related Work section: "namerly" -> "namely"