Re: [Rd] .onLoad, packageStartupMessage, and R CMD check

2021-11-05 Thread Michael Chirico via R-devel
Examining more closely, it's a NOTE produced by R CMD check --
originally I had thought it was a WARNING, which I think would have
been too strong for this case. A NOTE actually seems fine, on second
thought.

For a tiny bit of context, it's common for us to issue messaging
around some state initialization, which has to happen after some
(ex-ante unknown) set of packages are loaded. It's important to do so
whether or not the package is attached, so the proviso in .onLoad()
indeed makes the most sense.

Thanks!

On Thu, Nov 4, 2021 at 1:02 PM Gabriel Becker  wrote:
>
> Hi Michael,
>
> Indeed, just to elaborate further on what I believe Duncan's point is, can 
> you give any examples, "dire" or not, that are appropriate when the package 
> is loaded but not attached (ie none of its symbols are visible to the user 
> without using :::)?
>
> The only things I can think of are a package that changes the behavior of 
> other, attached package code, such as conflicted. Doing so is very much an 
> anti-pattern imo generally, with something like conflicted being an 
> (arguable) exception. And that's assuming conflicted even works/does anything 
> when loaded but not attached (I have not confirmed whether thats the case or 
> not). That or a package that is at end-of-life and is or soon will be 
> unsupported entirely.
>
> The examples don't need to be yours, per se, if you know what those pushing 
> back against your linter were using messages from .onLoad for...
>
> Best,
> ~G
>
>
>
> On Thu, Nov 4, 2021 at 12:37 PM Duncan Murdoch  
> wrote:
>>
>> On 04/11/2021 2:50 p.m., Michael Chirico via R-devel wrote:
>> > I wrote a linter to stop users from using packageStartupMessage() in
>> > their .onLoad() hook because of the R CMD check warning it triggers:
>> >
>> > https://github.com/wch/r-source/blob/8b6625e39cd62424dc23399dade37f20fa8afa91/src/library/tools/R/QC.R#L5167
>> >
>> > However, this received some pushback which I ultimately agree with,
>> > and moreover ?.onLoad seems to agree as well:
>> >
>> >> Loading a namespace should where possible be silent, with startup
>> > messages given by \code{.onAttach}. These messages (**and any essential
>> > ones from \code{.onLoad}**) should use \code{\link{packageStartupMessage}}
>> > so they can be silenced where they would be a distraction.
>> >
>> > **emphasis** mine. That is, if we think some message is _essential_ to
>> > print during loadNamespace(), we are told to use
>> > packageStartupMessage().
>> >
>> > Should we remove this R CMD check warning?
>>
>> The help page doesn't define what an "essential" message would be, but I
>> would assume it's a message about some dire condition, not just "Hi! I
>> just got loaded!".  So I think a note or warning would be appropriate,
>> but not an error.
>>
>> Do you have an example of something that should routinely print, but
>> that triggers a warning when checked?
>>
>> Duncan Murdoch
>>
>> __
>> R-devel@r-project.org mailing list
>> https://stat.ethz.ch/mailman/listinfo/r-devel

__
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel


Re: [Rd] .onLoad, packageStartupMessage, and R CMD check

2021-11-04 Thread Gabriel Becker
Hi Michael,

Indeed, just to elaborate further on what I believe Duncan's point is, can
you give any examples, "dire" or not, that are appropriate when the package
is loaded but not attached (ie none of its symbols are visible to the user
without using :::)?

The only things I can think of are a package that changes the behavior of
other, attached package code, such as conflicted. Doing so is very much an
anti-pattern imo generally, with something like conflicted being an
(arguable) exception. And that's assuming conflicted even works/does
anything when loaded but not attached (I have not confirmed whether thats
the case or not). That or a package that is at end-of-life and is or soon
will be unsupported entirely.

The examples don't need to be yours, per se, if you know what those pushing
back against your linter were using messages from .onLoad for...

Best,
~G



On Thu, Nov 4, 2021 at 12:37 PM Duncan Murdoch 
wrote:

> On 04/11/2021 2:50 p.m., Michael Chirico via R-devel wrote:
> > I wrote a linter to stop users from using packageStartupMessage() in
> > their .onLoad() hook because of the R CMD check warning it triggers:
> >
> >
> https://github.com/wch/r-source/blob/8b6625e39cd62424dc23399dade37f20fa8afa91/src/library/tools/R/QC.R#L5167
> >
> > However, this received some pushback which I ultimately agree with,
> > and moreover ?.onLoad seems to agree as well:
> >
> >> Loading a namespace should where possible be silent, with startup
> > messages given by \code{.onAttach}. These messages (**and any essential
> > ones from \code{.onLoad}**) should use
> \code{\link{packageStartupMessage}}
> > so they can be silenced where they would be a distraction.
> >
> > **emphasis** mine. That is, if we think some message is _essential_ to
> > print during loadNamespace(), we are told to use
> > packageStartupMessage().
> >
> > Should we remove this R CMD check warning?
>
> The help page doesn't define what an "essential" message would be, but I
> would assume it's a message about some dire condition, not just "Hi! I
> just got loaded!".  So I think a note or warning would be appropriate,
> but not an error.
>
> Do you have an example of something that should routinely print, but
> that triggers a warning when checked?
>
> Duncan Murdoch
>
> __
> R-devel@r-project.org mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel
>

[[alternative HTML version deleted]]

__
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel


Re: [Rd] .onLoad, packageStartupMessage, and R CMD check

2021-11-04 Thread Duncan Murdoch

On 04/11/2021 2:50 p.m., Michael Chirico via R-devel wrote:

I wrote a linter to stop users from using packageStartupMessage() in
their .onLoad() hook because of the R CMD check warning it triggers:

https://github.com/wch/r-source/blob/8b6625e39cd62424dc23399dade37f20fa8afa91/src/library/tools/R/QC.R#L5167

However, this received some pushback which I ultimately agree with,
and moreover ?.onLoad seems to agree as well:


Loading a namespace should where possible be silent, with startup

messages given by \code{.onAttach}. These messages (**and any essential
ones from \code{.onLoad}**) should use \code{\link{packageStartupMessage}}
so they can be silenced where they would be a distraction.

**emphasis** mine. That is, if we think some message is _essential_ to
print during loadNamespace(), we are told to use
packageStartupMessage().

Should we remove this R CMD check warning?


The help page doesn't define what an "essential" message would be, but I 
would assume it's a message about some dire condition, not just "Hi! I 
just got loaded!".  So I think a note or warning would be appropriate, 
but not an error.


Do you have an example of something that should routinely print, but 
that triggers a warning when checked?


Duncan Murdoch

__
R-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-devel