Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BiocManager::install() not installing package if available version is lower than current #128

Open
hpages opened this issue Jan 21, 2022 · 11 comments

Comments

@hpages
Copy link
Contributor

hpages commented Jan 21, 2022

I have SQLDataFrame 1.9.1:

packageVersion("SQLDataFrame")
# [1] ‘1.9.1’

and the version of SQLDataFrame currently available in BioC 3.15 is 1.9.0:

library(BiocManager)
available.packages(repos=BiocManager::repositories())["SQLDataFrame", "Version"]
# [1] "1.9.0"

But if I run BiocManager::install() I get:

BiocManager::install("SQLDataFrame")
# Bioconductor version 3.15 (BiocManager 1.30.16), R Under development (unstable)
#   (2021-10-25 r81105)
# Warning message:
# package(s) not installed when version(s) same as current; use `force = TRUE` to
#   re-install: 'SQLDataFrame' 

It seems to me that BiocManager::install("somePackage") should just re-install by default (i.e. without the need to use force=TRUE) when the available version is different from the installed version, including when it's lower than the installed version.

If we don't want that, then the warning message would need to be corrected to say something like:

package(s) not installed when version(s) same as (or lower than) current; use force = TRUE to re-install: 'somePackage'

FWIW I ran into this issue in the context of BiocManager::valid():

> BiocManager::valid()

* sessionInfo()

R Under development (unstable) (2021-10-25 r81105)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Ubuntu 21.10

Matrix products: default
BLAS:   /home/hpages/R/R-4.2.r81105/lib/libRblas.so
LAPACK: /home/hpages/R/R-4.2.r81105/lib/libRlapack.so

locale:
 [1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C              
 [3] LC_TIME=en_GB              LC_COLLATE=en_US.UTF-8    
 [5] LC_MONETARY=en_US.UTF-8    LC_MESSAGES=en_US.UTF-8   
 [7] LC_PAPER=en_US.UTF-8       LC_NAME=C                 
 [9] LC_ADDRESS=C               LC_TELEPHONE=C            
[11] LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C       

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

other attached packages:
[1] Spectra_1.5.6        ProtGenerics_1.27.2  BiocParallel_1.29.12
[4] S4Vectors_0.33.10    BiocGenerics_0.41.2  BiocManager_1.30.16 

loaded via a namespace (and not attached):
[1] MASS_7.3-55       compiler_4.2.0    IRanges_2.29.1    parallel_4.2.0   
[5] tools_4.2.0       fs_1.5.2          MsCoreUtils_1.7.1 clue_0.3-60      
[9] cluster_2.1.2    

Bioconductor version '3.15'

  * 0 packages out-of-date
  * 3 packages too new

create a valid installation with

  BiocManager::install(c(
    "coMET", "GenomeInfoDb", "SQLDataFrame"
  ), update = TRUE, ask = FALSE)

more details: BiocManager::valid()$too_new, BiocManager::valid()$out_of_date

Warning message:
0 packages out-of-date; 3 packages too new 

But when I tried to create a valid installation by copy/paste'ing the suggested command I got:

>   BiocManager::install(c(
+     "coMET", "GenomeInfoDb", "SQLDataFrame"
+   ), update = TRUE, ask = FALSE)
Bioconductor version 3.15 (BiocManager 1.30.16), R Under development (unstable)
  (2021-10-25 r81105)
Warning message:
package(s) not installed when version(s) same as current; use `force = TRUE` to
  re-install: 'coMET' 'GenomeInfoDb' 'SQLDataFrame' 

Thanks,
H.

@LiNk-NY
Copy link
Contributor

LiNk-NY commented Jan 21, 2022

Thanks Hervé! I will update the message.

@LiNk-NY
Copy link
Contributor

LiNk-NY commented Jan 21, 2022

@hpages
Copy link
Contributor Author

hpages commented Jan 21, 2022

Thanks Marcel. If BiocManager::install() is not going to reinstall those packages by default then the command suggested by BiocManager::valid() would need to have force=TRUE.

@hpages
Copy link
Contributor Author

hpages commented Jan 21, 2022

Also the documentation for the force argument should clarify what happens in that case. Right now it kind of suggests that using force=TRUE is only needed for reinstalling packages that are currently up-to-date. Are installed packages that are too new considered "up-to-date"?

@LiNk-NY
Copy link
Contributor

LiNk-NY commented Jan 22, 2022

Thanks Marcel. If BiocManager::install() is not going to reinstall those packages by default then the command suggested by BiocManager::valid() would need to have force=TRUE.

We would have to create a mechanism to separate packages that need to be installed with force=TRUE vs not. "too_new" packages do not happen often unless users are heavy GitHub package installers and it may be more trouble than it is worth to let the few users know of this distinction..

Perhaps Martin @mtmorgan has some thoughts ?

Are installed packages that are too new considered "up-to-date"?

They are not considered up-to-date and I will update the documentation.
Update: 35e3d4d

@mtmorgan
Copy link
Collaborator

I like the current behavior, so changing the documentation of force=TRUE to clarify that it can also be used to replace too-new packages, and updating the valid() help text. I'm not seeing the need for new functionality beyond that?

@hpages
Copy link
Contributor Author

hpages commented Jan 22, 2022

Isn't it that all the packages that BiocManager::valid() report as being either out-of-date or too new can be installed with force=TRUE? I understand that force=TRUE is not needed to install out-of-date packages but I don't see how it would hurt to use it in that case (it would actually make no difference).

Furthermore, you could add force=TRUE to the suggested install command only when one of the packages to update is too new. That way you're not making the command unnecessarily more complicated when all the packages to update are out-of-date, which is the most common use case.

Thanks

@LiNk-NY
Copy link
Contributor

LiNk-NY commented Jan 24, 2022

Thanks!
I've added this here: 66e96bb

@hpages
Copy link
Contributor Author

hpages commented Jan 24, 2022

Great! Thanks Marcel.

Just a note about the use of character(0) vs "" in the context of paste()/paste0():

paste0("X", character(0))
paste0("X", "")

Both work and do the same thing. However I would argue that the former is less readable and works only "by accident". It behaves like the latter only because paste0() doesn't follow the recycling scheme that is used by almost everything else. For example, when adding 2 numbers, using numeric(0) won't produce the same result as using 0:

5 + numeric(0)  # result is numeric(0)
5 + 0           # result is 5

So if the intention is to not add anything, 0 must be used, not numeric(0).

The empty string ("") is to string concatenation what 0 is to addition.

H.

@LiNk-NY
Copy link
Contributor

LiNk-NY commented Jan 24, 2022

Can we both be happy with character(1L)? 😁
Thanks for the note!
0238166

@hpages
Copy link
Contributor Author

hpages commented Jan 25, 2022

I dunnow. I doubt this is how you would teach a 12-year old how to represent an empty string in R 😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants