BiocManager icon indicating copy to clipboard operation
BiocManager copied to clipboard

Why is BiocManager emitting messages on package load?

Open LTLA opened this issue 4 years ago • 8 comments
trafficstars

The change in 1fed5289f00871b8b4d42736c7c329378da21627 is causing emission of annoying messages in all packages that Import BiocManager. For example, BiocStyle imports BiocManager, and this is causing all my vignettes and reports generated in an interactive session to contain this message:

Bioconductor version 3.13 (BiocManager 1.30.12), ?BiocManager::install for help

in every location where I have library(BiocStyle). I didn't bother to suppressPackageStartupMessages() because BiocStyle was not emitting any messages previously. One could argue that I should suppress these messages, but then I would say that I shouldn't have to see this message in the first place, especially if I'm not using BiocManager directly. Why the chattiness?

LTLA avatar Apr 08 '21 07:04 LTLA

I just noticed that too earlier today after doing library(BiocStyle) on one of the build machines. Looks like the .onAttach hook was replaced by an .onLoad hook (see commit 1fed5289f00871b8b4d42736c7c329378da21627). Not sure why.

As a general rule I don't think .onLoad hooks should print messages.

hpages avatar Apr 08 '21 08:04 hpages

Any insights, Martin? @mtmorgan Thanks!

LiNk-NY avatar Apr 08 '21 18:04 LiNk-NY

The banner can be useful in bug reports, and the 'best practice' recommendation for use (e.g., BiocManager::version()) doesn't attach the package. So it made sense to have the message in .onLoad rather than .onAttach.

I'm not sure why a package would import BiocManager: package installation shouldn't be something a package attempts to 'do' for the user.

mtmorgan avatar Apr 08 '21 22:04 mtmorgan

BiocStyle imports BiocManager to get version(), so that Biocpkg() and friends can point to the correct URL.

LTLA avatar Apr 08 '21 23:04 LTLA

Maybe best practice should be to explicitly library() the package before using it interactively? That's kind of the standard thing to do, for any package.

hpages avatar Apr 09 '21 05:04 hpages

I chose not to emphasize using library() because I wanted commands that the user sees to be fully resolved and therefore not subject to namespace collisions -- the natural interpretation of install(), version(), etc resolved with BiocManager::...() seem like a better choice than some name mangling bm_installl() etc.

I will revisit this so that version() etc returns an object that is a version and hence computable, but prints with information about package version etc. I will remove the startup message.

mtmorgan avatar Apr 09 '21 10:04 mtmorgan

You could condition the message based on how the package was loaded by looking at sys.calls()[1]. For instance, show it if user called it via a BiocManager::..., library(BiocManager), ...

HenrikBengtsson avatar Apr 13 '21 15:04 HenrikBengtsson

Thanks @HenrikBengtsson; I shy away from that level of complexity.

mtmorgan avatar Apr 13 '21 16:04 mtmorgan