search

ctlab / fgsea

Fast Gene Set Enrichment Analysis
Other
379 stars 67 forks source link

Depends: Rcpp #65

Closed pcarbo closed 4 years ago

pcarbo commented 4 years ago

It is highly unusual for Rcpp to be included in the Depends section of the DESCRIPTION. I may be wrong here, but I believe it should be listed instead in the Imports section. (It is correctly listed in LinkingTo.)

pcarbo commented 4 years ago

Also, unrelated to this, adding a Remotes entry to the DESCRIPTION file would help with automatically installing the Bioconductor dependencies; when I tried to build the vignette it didn't work until I manually installed a couple of the missing Bioconductor packages. See here for details: https://remotes.r-lib.org/articles/dependencies.html

assaron commented 4 years ago

Yes, it seems you are right. Having Rcpp in Depends is not uncommon, but Imports should be enough.

For the Remotes part, how exactly are installing fgsea. For building vignette the packages in Suggests should be installed, which can be easily done with devtools::install(..., dependencies=TRUE). I believe it works well with Bioconductor packages.

pcarbo commented 4 years ago

@assaron On its own, devtools doesn't know where to find the Bioconductor packages, so you need to add the Remotes: entry to tell it where to look for them. This is similar to packages that are only available on GitHub. (This is a minor issue that only comes up when trying to build the vignette.)

assaron commented 4 years ago

This is strange, I believe for some time devtools knows about Bioconductor dependencies, see for example the discussion and particular comment here: https://github.com/r-lib/devtools/issues/700#issuecomment-235127291

pcarbo commented 4 years ago

Yes, it is entirely possible that this has changed. They have made many changes to devtools in the past few years.

assaron commented 4 years ago

Rcpp is now in imports