Full integration with codetoolsBioc

[federicomarini]

... potentially with codetoolsBioc (https://github.com/Bioconductor/codetoolsBioC) becoming at all effects a part of Bioconductor. As of now, it only lives in Github and is listed in the Enhances for BiocCheck.

Why?
I find particularly useful the integration with BiocCheck when it automatically provides a bunch of meaningful (and in most of the cases working perfectly OOTB) namespace imports.

How?
Some aspects could be updated for codetoolsBioc: roxygen-based manual & handling of dependencies, potential switch to testthat for unit testing, ...

[hpages]

Let's please be cautious with codetoolsBioc namespace imports. I've really bad memories of seeing many packages with questionable NAMESPACE directives generated by codetoolsBioc automatic namespace generation feature, e.g. super selective imports from the methods package. These NAMESPACE files would almost always create problems in the long run and I had to fix dozens of them. I'd rather not go through this again.

[federicomarini]

Agreed, I also recall the strange behavior on methods.
It could probably still be useful for generating the first bulk of such directives - which often needs as you say some manual curation? I'm fine with either answer here 😊

[HenrikBengtsson]

I find particularly useful the integration with BiocCheck when it automatically provides a bunch of meaningful (and in most of the cases working perfectly OOTB) namespace imports.

FWIW, R CMD check produces WARNINGs with suggestions like:

Consider adding
  importFrom("methods", "is")
to your NAMESPACE file (and ensure that your DESCRIPTION Imports field
contains 'methods').

I'm not 100% sure under what circumstances these are picked up. It might be that it only recommends this for functions in base R packages.

[federicomarini]

Yes, the base (+ graphics or so) functions seem like they're the ones that get correctly suggested in that scope.

[federicomarini]

Happy to try out the full grooming to Bioc package. @federicomarini

[federicomarini]

I did start up with some code last week, for bringing the package to a "modern status".

• apologies for the not so full descriptions in the commit
• I was working on my master and not on a separate branch, I hope it is not so dramatic (did not expect any new commits coming in for this package)

The PR will be posted on the https://github.com/Bioconductor/codetoolsBioC repo directly

[hpages]

Hi Federico, Henrik,

Just to be clear on this, the importFrom("methods", "is") suggestion is unfortunately not a good one. See this recent discussion on the bioc-devel mailing list.

H.

[federicomarini]

Thanks @hpages for pointing this out!
I'm far from delving into the gears of codetoolsBioc internals, this really exceeds my current understanding of the whole thing 😛
My proposal in this case was to pimp up the package "to the Bioc level". I understand that this should be done with handfuls of care to have it return the best suggestions. I think mine was more an homage of the many times it turned out to be useful for my own packages ;)

[hpages]

I understand. One more thing that would need to be addressed in the process of pimping up the package "to the Bioc level" is writeNamespaceImports() use of importMethodsFrom when really importFrom should almost always be used instead. This is because what should really be imported are generic functions, not their particular methods. It still works to use importMethodsFrom but only because it has the side effect of also importing the generics. So it works only by chance and thanks to an obscure feature.

Using importMethodsFrom instead of importFrom occasionally leads to problems, especially when changes happen in "parent packages" (methods tend to move around a lot more than generic functions). I've fixed countless of NAMESPACE files in the past and I still fix some occasionally.

Unfortunately we still have about 350 software packages that still use importMethodsFrom in their NAMESPACE at the moment, and most of them should use importFrom instead. AFAICT situations where importMethodsFrom is actually the right thing to use are very rare.