Replace biocLite with BiocManager

trinker / pacman

A package management tools for R

311 stars 37 forks source link

Replace biocLite with BiocManager #110

Closed gadenbuie closed 5 years ago

gadenbuie commented 6 years ago

Implements #109:

Adds skip_bioc parameter to p_install() (default NULL) that resolves to TRUE if BiocManager is not installed.
Uses p_install() from try_bioc() to install BiocManager if not installed and skip_bioc = FALSE.
Uses BiocManager::install() to install from Bioconductor, which by default prompts user if updates are desired and session is interactive. Updates are suppressed otherwise.

Happy to tweak this PR as needed, feedback is welcome.

rorynolan commented 6 years ago

Hi @gadenbuie, Great work! I love pacman and came on here to suggest this too. I think pacman should just depend on BiocManager (using the Imports: field in DESCRIPTION) and then the default should be to install the package from bioc if its available there? What do you think? I can't think of any reason why installing from bioc should be turned off by default.

gadenbuie commented 6 years ago

Thanks @rorynolan. I think that including BiocManager in DESCRIPTION is necessary and something I can add. But I think it would be good to get input from @trinker or @Dasonk about whether BiocManager should be in Imports or Suggests.

Personally, after a second look at this PR, I'd like to put it in Imports and then simplify the code so that the default behavior is to (try to) install from Bioconductor if a package isn't found on CRAN. I was trying to accommodate this comment, but I feel it's probably best for the user for pacman to do its best to install the requested package.

Both pacman and BiocManager are very light-weight -- BiocManager only imports utils.

gadenbuie commented 6 years ago

On second thought, adding BiocManager to Imports really simplifies this PR considerably, so I went ahead and added those commits. As it stands, this PR now

Adds BiocManager to Imports and uses BiocManager::install() instead of BiocInstaller::biocLite()
Adds a try_bioconductor parameter to p_install() with default of TRUE.

rorynolan commented 6 years ago

Great! This is exactly how I would like it to be.

gadepallivs commented 6 years ago

Isn't it BiocManager::install(), can take care of package installation across CRAN, Bioconductor and github ? If so, what is the need for p_install() and p_install_gh() as 2 separate commands?

trinker commented 5 years ago

Awesome! Thanks for the PR.