Bioconductor / BiocParallel

Bioconductor facilities for parallel evaluation
https://bioconductor.org/packages/BiocParallel
65 stars 29 forks source link

Make 'codetools' a required package? #212

Closed HenrikBengtsson closed 2 years ago

HenrikBengtsson commented 2 years ago

Hi, I spotted that some packages forget to declare codetools under Suggests: when one of their dependencies use codetools conditionally. This is often not an issue on systems where "recommended" packages (e.g. codetools) are installed together with the base-R packages in the system package folder (= .Library).

However, on systems where the "recommended" packages are not installed centrally, these packages may fail. For example, I spotted that Bioconductor package beer (https://github.com/athchen/beer/issues/1) lacks an explicit dependency on codetools and therefore fail with Error in loadNamespace(x) : there is no package called ‘codetools’. In this case, codetools is needed because it hits BiocParallel:::.findVariables(), which uses codetools::findGlobals().

I don't know how commonly BiocParallel:::.findVariables() is used when using the BiocParallel package, but if it is quite likely that it called, and therefore codetools is used, would you consider importing from codetools rather than "only" Suggests:-ing it? That way, packages depending on BiocParallel are guaranteed to install also codetools.

PS. You can replicate the issue with beer by running R CMD check --as-cran ..., or _R_CHECK_DEPENDS_ONLY_=true R CMD check ..., while not having codetools in the central .Library folder.

Jiefei-Wang commented 2 years ago

Nice catch! I think we use BiocParallel:::.findVariables a lot internally, so this will be an issue for the system without the recommended packages. Either we should import the package or we reinvent the wheel. I'd like to see Martin's thought on it.

mtmorgan commented 2 years ago

Yes, previous changes make this code path a much more common occurrence, so I have moved codetools from Suggests to Imports. I think this introduces minimal additional dependencies for most users (where codetools is already installed).

HenrikBengtsson commented 2 years ago

Thanks