HelenaLC / CATALYST

Cytometry dATa anALYsis Tools
66 stars 31 forks source link

Please add 'uwot' to Suggests: of CATALYST #145

Closed HenrikBengtsson closed 4 years ago

HenrikBengtsson commented 4 years ago

Issue

Hi. While reverse-dependency checking matrixStats, I get an error that there is no package called ‘uwot’. This is because some function call of yours rely on some package that has uwot as a Suggests: package. If the package that uses uwot had it under Depends: or Imports:, this would just work.

Patch

The solution is to add the following CATALYST's DESCRIPTION file:

Suggests: uwot

Details

You can replicate this error by running R CMD check --as-cran (which revdepcheck::revdep_check() uses). This will make sure that the package is tested in a sandboxed environment where all packages explicitly declared are available to the test but no other package. This minimizes the risk of have packages in your personal package library "rescuing" the checks.
Adding uwot to Suggests, this will help R CMD check --as-cran making sure uwot is always for its test. If not, it will explicitly tell us it's missing.

In this case, when you run your checks, or when Bioconductor runs their checks, I suspect you use R CMD check and you already have uwot in your personal package library, so R CMD check will fall back to that installation.

FYI, I've seen this mistake in CATALYST for several years, so it's nothing new.

R CMD check errors

*   checking examples ... ERROR
...
Running examples in ‘CATALYST-Ex.R’ failed
The error most likely occurred in:

> ### Name: plotDR
> ### Title: Plot reduced dimensions
> ### Aliases: plotDR
> 
> ### ** Examples
> 
> # construct SCE & run clustering
> data(PBMC_fs, PBMC_panel, PBMC_md)
> sce <- prepData(PBMC_fs, PBMC_panel, PBMC_md)
> 
> # run clustering & dimension reduction
> sce <- cluster(sce)
o running FlowSOM clustering...
o running ConsensusClusterPlus metaclustering...
> sce <- runDR(sce, dr = "UMAP", cells = 100)
Error in loadNamespace(name) : there is no package called ‘uwot’
Calls: runDR ... loadNamespace -> withRestarts -> withOneRestart -> doWithOneRestart
Execution halted
```
and

...

The following objects are masked from 'package:matrixStats':

  colMaxs, colMins, colRanges, rowMaxs, rowMins, rowRanges

The following objects are masked from 'package:base':

  aperm, apply, rowsum

test_check("CATALYST") ── 1. Error: plotDR() (@test_plotting-differential.R#259) ───────────────────── error in evaluating the argument 'x' in selecting a method for function 'reducedDim': there is no package called 'uwot' Backtrace:

══ testthat results ═══════════════════════════════════════════════════════════ [ OK: 722 | SKIPPED: 0 | WARNINGS: 1 | FAILED: 1 ]

  1. Error: plotDR() (@test_plotting-differential.R#259)

    Error: testthat unit tests failed Execution halted

HelenaLC commented 4 years ago

Thank you Hendrik for pointing this out and the detailed reasoning behind your suggestion.

Believe it or not, I was actually fully aware of this, and I don't remember at this point why uwot wasn't included under Suggests:. I think it must have simply been the fact that BiocCheck passed without it, which is a really bad reason... My apologies!

I have added it to the devel version, as I am hesitant to touch the release branch at this point (will be frozen in ~2 weeks). It will become available with the release end of next month.