pacman icon indicating copy to clipboard operation
pacman copied to clipboard

BiocInstaller runs only if it's not installed

Open Dasonk opened this issue 9 years ago • 5 comments
trafficstars

https://github.com/trinker/pacman/blob/master/R/p_install.R#L57

The comments before that line imply that you want us to check if a package is on bioconductor IF BiocInstaller is installed. Right now it will try to install from Bioconductor no matter what. Also was there something that requires using eval(parse(text=...))) as opposed to passing the values straight into biocLite?

Dasonk avatar Sep 13 '16 17:09 Dasonk

I think the comments are from a deranged fool. I actually don't understand them. From what I can tell it does this or this was what I intended it to do:

  1. checks if BiocInstaller is already installed
  2. If NOT then install it
  3. Install the bioconductor based packages (assumes BiocInstaller installed or was available)

The eval gets around adding it as a dependency in CRAN check and might be to allow quotes around the package name. Not 100% sure about this without playing directly. But mainly, if we use BiocInstaller::biocLite('%s', suppressUpdates=TRUE) directly we'll get a CRAN check warning for not including it in the dependencies.

trinker avatar Sep 14 '16 01:09 trinker

Does it give a warning if we add it to suggests? Because it's really the use case for suggests. But do we really want to go installing BiocInstaller for everybody? Because if somebody misspells a package name and it can be found in CRAN then right now it will install BiocInstaller and then check if it's in bioconductor. I mean we aim for convenience but that can tie up the machine for a while the first time somebody accidentally misspells a package name. Maybe we could add a package option or a function option to decide if to check bioconductor?

Dasonk avatar Sep 14 '16 13:09 Dasonk

Bump

Dasonk avatar Apr 26 '17 17:04 Dasonk

THis PR is related: https://github.com/trinker/pacman/pull/91

The current implementation installs BiocInstaller every time p_install doesn't find a package on CRAN. This is in response to an off thread point @Dasonk had about the potential for change in this package and needing a fresh install to be certain it is doing what it is supposed to be doing. However, this exasperates the point you make above about tying up the machine, not one time but every time. Thoughts?

trinker avatar May 01 '17 01:05 trinker

Maybe we have it off by default and print a message telling them that bioconductor isn't being checked and give them the option to set if they want bioconductor to be checked?

I know we said 'on by default' for sake of convenience but I guess we should weight that against how much extra overhead we might be creating that the user doesn't necessarily know is going on.

Dasonk avatar May 02 '17 12:05 Dasonk