trinker / pacman

A package management tools for R
313 stars 37 forks source link

p_install vignette doesn't work #67

Open cswarth opened 9 years ago

cswarth commented 9 years ago

The pacman vignette says,

p_install(dbConnect, qdap, reports)

When I try this I get,

> p_install(dbConnect, qdap, reports)
Error in p_install(dbConnect, qdap, reports) : object 'qdap' not found

This can be solved by passing a vector to p_install,

> p_install(c(dbConnect, qdap, reports))
Installing packages into ‘/home/user/R/library’
(as ‘lib’ is unspecified)
also installing the dependencies ‘data.table’, ‘openNLPdata’, ‘RMySQL’, ‘gWidgets’, ‘qdapDictionaries’, ‘qdapRegex’, ‘qdapTools’, ‘gdata’, ‘gender’, ‘NLP’, ‘openNLP’, ‘plotrix’, ‘tm’, ‘venneuler’, ‘wordcloud’
...

I don't know whether the bug is in the vignette or in the code.

> sessionInfo()
R version 3.2.2 (2015-08-14)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 14.04.2 LTS

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

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

other attached packages:
[1] pacman_0.3.0
cswarth commented 9 years ago

Actually I think p_install may be only taking a single argument, not a vector.
Passing a vector seems to work but eventually you get this error,

3: package ‘c’ is not available (for R version 3.2.2) 

presumably indicating it parsed 'c()' as a package name. Only way I have gotten p_install to work is to pass a single package name at a time.

Dasonk commented 9 years ago

Yes p_install is only for single packages. p_load is really the workhorse function in the package. It's meant to load the packages (and install them beforehand if necessary) but I typically use p_load to install packages when needed. p_load takes multiple inputs.

This does seem to be an issue with our documentation. It probably would be better for us to allow p_install to take multiple inputs in a fashion similar to p_load though.

pgensler commented 7 years ago

Yes, I think it would definitely be helpful to have p_install take multiple inputs so that the functions are a bit more consistent.

Dasonk commented 7 years ago

I'm surprised we haven't made this change yet but one thing I'm noticing is that to make the change we would need to change p_install to take dots as the first parameter. This would require all of the other parameters to be named and to be spelled out in full (no using 'char' as a shortcut for 'character.only'). This could possibly break some workflows.

With that said our vignette is still out of date with how p_install actually can be used. We could start by updating the vignette. A long term plan if we didn't want to surprise anybody and create errors would be to add a package startup message or message when p_install is getting called to tell people about a change that will be made in the next version? Kind of like deprecating something but in this case it's not really deprecated... just modified.

@trinker Any thoughts?

pgensler commented 7 years ago

It would be nice to be able to also have p_install state when it starts the install, and when it completes, which I think? it's supposed to do? devtools::install_cran does this, so maybe simply wrapping that could be an option?