alanocallaghan / scater

Clone of the Bioconductor repository for the scater package.
https://bioconductor.org/packages/devel/bioc/html/scater.html
95 stars 40 forks source link

Use import or importFrom for `pheatmap` #169

Closed ycl6 closed 2 years ago

ycl6 commented 2 years ago

The pheatmap function/package is used to create heatmaps in the plotHeatmap() and plotGroupedHeatmap(), but pheatmap() is called using ::, thus not part of the packages in Imports in DESCRIPTION.

When user installs scater, pheatmap was not checked to see if it has been installed even though plotHeatmap() and plotGroupedHeatmap() require pheatmap to work. Resulting in error:

Error: there is no package called ‘pheatmap’
alanocallaghan commented 2 years ago

There's a bunch of functionality in scater that requires external libraries that are only soft required. Even uwot is suggested rather than required; I guess the idea is to make a barebones install faster and if you want extra bells and whistles just install the extra packages.

I'll consider making everything a hard requirement

ycl6 commented 2 years ago

Hi @Alanocallaghan Thanks for considering this.

pheatmap is one of those packages that'll be installed one way or another over time. I only found this out when I was putting a Shiny App that's working on my computer onto another server that I set up with bare minimum R packages required to run, including scater to use its plot functions.

I'll close this now.

LTLA commented 2 years ago

For some context, scater used to be a dependency for packages that didn't need its plotting functionalities, just its normalization functions and such. As such, all packages required for plotting were pushed to Suggests so as to avoid burdening scater's reverse dependencies with more installation requirements. Now that the non-plotting utilities have been split off into scuttle, it should be okay to move some or all of the plotting dependencies into Imports.