R devel - Apr 2015 - R CMD check and missing imports from base packages

On Wed, Apr 29, 2015 at 1:45 PM, Gabriel Becker <gmbecker at ucdavis.edu>
wrote:
> Gabor,
>
> To play devil's advocate a bit,  why not just have the package formally
> import the functions it wants to use (or the whole package if that is
> easier)?
>
This is exactly my goal. And to facilitate this, R CMD check could remind
you if you forgot to do the formal import.

You don't want to require people to import things like the
assignment
> operator, or "if", or a bunch of other things in the base package
(and
> probably not the stuff in grDevices either, though from your description
> they would in principle need to do that now).
>
I am not talking about the 'base' package, only the other base packages.
(The base package is special, it is searched before the attached packages,
so it is probably fine.)

Yes, I suggest people import stuff from grDevices explicitly. Because if
they don't, their package might break if used together with other packages.
And this is not a theoretical issue, in the past couple of weeks I have
seen this happening three times.

But why should stats not require an import if you want to guarantee
that
> you get the density function from stats and not from somewhere else?
Isn't
> that what ImportFrom is for? Is the reason that it is loaded automatically?
>
That's exactly what I am saying, sorry if it was not clear. I want to
"require" format imports from base packages (except from _the_ base
package, maybe).

Yes, people can do this already. But why not help them with a NOTE if they
don't know that this is good practice, or they just simply forget?

Gabor

>
> Best,
> ~G
>
> On Wed, Apr 29, 2015 at 10:00 AM, G?bor Cs?rdi <csardi.gabor at
gmail.com>
> wrote:
>
>> On Wed, Apr 29, 2015 at 12:53 PM, Winston Chang <winstonchang1 at
gmail.com>
>> wrote:
>>
>> > On Tue, Apr 28, 2015 at 3:04 PM, G?bor Cs?rdi <csardi.gabor at
gmail.com>
>> > wrote:
>> > >
>> > >
>> > > E.g. if package 'ggplot2' uses
'stats::density()', and package
>> 'igraph'
>> > > also defines 'density()', and 'igraph' is on
the search path, then
>> > > 'ggplot2' will call 'igraph::density()'
instead of 'stats::density()'.
>> >
>> >
>> > Just to be clear: you mean that this happens when ggplot2 contains
a
>> > call like 'density()', not 'stats::density()' (but
the intention is to
>> > call stats::density()), right?
>> >
>>
>> Yes, exactly as you say, I am sorry for the confusion. This is actually
a
>> real example:
>> https://github.com/hadley/ggplot2/issues/1041
>>
>> Gabor
>>
>>
>> >
>> > -Winston
>> >
>>
>>         [[alternative HTML version deleted]]
>>
>> ______________________________________________
>> R-devel at r-project.org mailing list
>> https://stat.ethz.ch/mailman/listinfo/r-devel
>>
>
>
>
> --
> Gabriel Becker, PhD
> Computational Biologist
> Bioinformatics and Computational Biology
> Genentech, Inc.
>
	[[alternative HTML version deleted]]

On 29/04/2015 1:57 PM, G?bor Cs?rdi wrote:
> On Wed, Apr 29, 2015 at 1:45 PM, Gabriel Becker <gmbecker at
ucdavis.edu>
> wrote:
>
> > Gabor,
> >
> > To play devil's advocate a bit,  why not just have the package
formally
> > import the functions it wants to use (or the whole package if that is
> > easier)?
> >
>
> This is exactly my goal. And to facilitate this, R CMD check could remind
> you if you forgot to do the formal import.
>
> You don't want to require people to import things like the assignment
> > operator, or "if", or a bunch of other things in the base
package (and
> > probably not the stuff in grDevices either, though from your
description
> > they would in principle need to do that now).
> >
>
> I am not talking about the 'base' package, only the other base
packages.
> (The base package is special, it is searched before the attached packages,
> so it is probably fine.)
>
> Yes, I suggest people import stuff from grDevices explicitly. Because if
> they don't, their package might break if used together with other
packages.
> And this is not a theoretical issue, in the past couple of weeks I have
> seen this happening three times.
>
> But why should stats not require an import if you want to guarantee that
> > you get the density function from stats and not from somewhere else?
Isn't
> > that what ImportFrom is for? Is the reason that it is loaded
automatically?
> >
>
> That's exactly what I am saying, sorry if it was not clear. I want to
> "require" format imports from base packages (except from _the_
base
> package, maybe).
>
> Yes, people can do this already. But why not help them with a NOTE if they
> don't know that this is good practice, or they just simply forget?
I suspect the reason for this is historical:  at the time that the 
current warning was added, it would have flagged too many packages as 
problematic.  People do complain when base R makes changes that force 
them to change their packages.  Perhaps that decision should be 
reconsidered now.

Duncan Murdoch

All,

Here are two ideas on this:

1. have R CMD check show how every external function reference gets 
resolved.
2. have R CMD check warn anytime there is a potential name conflict, e.g. 
density( ) coming from either igraph or stats, and show how it was 
resolved.

Either could be an option.  I guess there are potential problems with 
conditional loading of libraries where a function could be  resolved 
differently depending on some decision made at run time, but 1. or 2. 
might be useful for the standard case.

John 
 ..............................................................
 John P. Nolan
 Math/Stat Department
 227 Gray Hall,   American University
 4400 Massachusetts Avenue, NW
 Washington, DC 20016-8050

 jpnolan at american.edu       voice: 202.885.3140 
 web: academic2.american.edu/~jpnolan
 ..............................................................



From:   G?bor Cs?rdi <csardi.gabor at gmail.com>
To:     Gabriel Becker <gmbecker at ucdavis.edu>, 
Cc:     "r-devel at r-project.org" <r-devel at r-project.org>
Date:   04/29/2015 01:57 PM
Subject:        Re: [Rd] R CMD check and missing imports from base 
packages
Sent by:        "R-devel" <r-devel-bounces at r-project.org>



On Wed, Apr 29, 2015 at 1:45 PM, Gabriel Becker <gmbecker at ucdavis.edu>
wrote:
> Gabor,
>
> To play devil's advocate a bit,  why not just have the package formally
> import the functions it wants to use (or the whole package if that is
> easier)?
>
This is exactly my goal. And to facilitate this, R CMD check could remind
you if you forgot to do the formal import.

You don't want to require people to import things like the
assignment
> operator, or "if", or a bunch of other things in the base package
(and
> probably not the stuff in grDevices either, though from your description
> they would in principle need to do that now).
>
I am not talking about the 'base' package, only the other base packages.
(The base package is special, it is searched before the attached packages,
so it is probably fine.)

Yes, I suggest people import stuff from grDevices explicitly. Because if
they don't, their package might break if used together with other 
packages.
And this is not a theoretical issue, in the past couple of weeks I have
seen this happening three times.

But why should stats not require an import if you want to guarantee
that
> you get the density function from stats and not from somewhere else? 
Isn't
> that what ImportFrom is for? Is the reason that it is loaded 
automatically?
>
That's exactly what I am saying, sorry if it was not clear. I want to
"require" format imports from base packages (except from _the_ base
package, maybe).

Yes, people can do this already. But why not help them with a NOTE if they
don't know that this is good practice, or they just simply forget?

Gabor

>
> Best,
> ~G
>
> On Wed, Apr 29, 2015 at 10:00 AM, G?bor Cs?rdi <csardi.gabor at
gmail.com>
> wrote:
>
>> On Wed, Apr 29, 2015 at 12:53 PM, Winston Chang 
<winstonchang1 at gmail.com>
>> wrote:
>>
>> > On Tue, Apr 28, 2015 at 3:04 PM, G?bor Cs?rdi 
<csardi.gabor at gmail.com>
>> > wrote:
>> > >
>> > >
>> > > E.g. if package 'ggplot2' uses
'stats::density()', and package
>> 'igraph'
>> > > also defines 'density()', and 'igraph' is on
the search path, then
>> > > 'ggplot2' will call 'igraph::density()'
instead of
'stats::density()'.
>> >
>> >
>> > Just to be clear: you mean that this happens when ggplot2 contains
a
>> > call like 'density()', not 'stats::density()' (but
the intention is
to
>> > call stats::density()), right?
>> >
>>
>> Yes, exactly as you say, I am sorry for the confusion. This is actually
a
>> real example:
>> https://github.com/hadley/ggplot2/issues/1041
>>
>> Gabor
>>
>>
>> >
>> > -Winston
>> >
>>
>>         [[alternative HTML version deleted]]
>>
>> ______________________________________________
>> R-devel at r-project.org mailing list
>> https://stat.ethz.ch/mailman/listinfo/r-devel
>>
>
>
>
> --
> Gabriel Becker, PhD
> Computational Biologist
> Bioinformatics and Computational Biology
> Genentech, Inc.
>
                 [[alternative HTML version deleted]]

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


	[[alternative HTML version deleted]]

On Wed, Apr 29, 2015 at 3:28 PM, John Nolan <jpnolan at american.edu>
wrote:
[...]
> 1. have R CMD check show how every external function reference gets
> resolved.
>
That's not possible, because it depends on the currently attached packages,
and even on the order of their attachment.

> 2. have R CMD check warn anytime there is a potential name conflict, e.g.
> density( ) coming from either igraph or stats, and show how it was
resolved.
>
Also not possible in practice, because it would need to check too many
packages. But it would not be a good solution in my opinion, anyway,
because you probably don't want people having to deal with these issues. I
surely don't. For the developer of a single package, it should be
completely irrelevant how many other packages use the same function names.

Gabor

[...]

	[[alternative HTML version deleted]]

On Wed, Apr 29, 2015 at 3:24 PM, Duncan Murdoch <murdoch.duncan at
gmail.com>
wrote:
[...]
> Yes, people can do this already. But why not help them with a NOTE if they
>> don't know that this is good practice, or they just simply forget?
>>
>
> I suspect the reason for this is historical:  at the time that the current
> warning was added, it would have flagged too many packages as problematic.
> People do complain when base R makes changes that force them to change
> their packages.  Perhaps that decision should be reconsidered now.

Thanks for considering this. I definitely think that with the number of
packages increasing, sooner or later this will be a (more) serious issue.
In some cases it is also hard(er) to work around with detaching and
re-attaching packages.

I think a NOTE would be probably enough, because probably a lot of packages
have these issues.

Also, people would not need to change their package _now_. Only if and when
they submit new versions. But that's CRAN's decision, not mine.

Gabor

[...]

	[[alternative HTML version deleted]]