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

When a symbol in a package is resolved, R looks into the package's
environment, and then into the package's imports environment. Then, if the
symbol is still not resolved, it looks into the base package. So far so
good.

If still not found, it follows the 'search()' path, starting with the
global environment and then all attached packages, finishing with base and
recommended packages.

This can be a problem if a package uses a function from a base package, but
it does not formally import it via the NAMESPACE file. If another package
on the search path also defines a function with the same name, then this
second function will be called.

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()'.

I think that for a better solution, either
1) the search path should not be used at all to resolve symbols in
packages, or
2) only base packages should be searched.

I realize that this is something that is not easy to change, especially 1)
would break a lot of packages. But maybe at least 'R CMD check' could
report these cases. Currently it reports missing imports for non-base
packages only. Is it reasonable to have a NOTE for missing imports from
base packages as well?

[As usual, please fix me if I am missing or misunderstood something.]

Thank you, Best,
Gabor

	[[alternative HTML version deleted]]

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?

-Winston

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]]

On 04/28/2015 01:04 PM, G?bor Cs?rdi wrote:
> When a symbol in a package is resolved, R looks into the package's
> environment, and then into the package's imports environment. Then, if
the
> symbol is still not resolved, it looks into the base package. So far so
> good.
>
> If still not found, it follows the 'search()' path, starting with
the
> global environment and then all attached packages, finishing with base and
> recommended packages.
>
> This can be a problem if a package uses a function from a base package, but
> it does not formally import it via the NAMESPACE file. If another package
> on the search path also defines a function with the same name, then this
> second function will be called.
>
> 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()'.
stats::density() is an S3 generic, so igraph would define an S3 method, right? 
And in general a developer would avoid masking a function in a base package, so 
as not to require the user to distinguish between stats::density() and 
igraph::density(). Maybe the example is not meant literally.

Being able to easily flag non-imported, non-base symbols would definitely 
improve the robustness of package code, even if not helping the end user 
disambiguate duplicate symbols.

Martin Morgan
>
> I think that for a better solution, either
> 1) the search path should not be used at all to resolve symbols in
> packages, or
> 2) only base packages should be searched.
>
> I realize that this is something that is not easy to change, especially 1)
> would break a lot of packages. But maybe at least 'R CMD check'
could
> report these cases. Currently it reports missing imports for non-base
> packages only. Is it reasonable to have a NOTE for missing imports from
> base packages as well?
>
> [As usual, please fix me if I am missing or misunderstood something.]
>
> Thank you, Best,
> Gabor
>
> 	[[alternative HTML version deleted]]
>
> ______________________________________________
> R-devel at r-project.org mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel
>

-- 
Computational Biology / Fred Hutchinson Cancer Research Center
1100 Fairview Ave. N.
PO Box 19024 Seattle, WA 98109

Location: Arnold Building M1 B861
Phone: (206) 667-2793

On Wed, Apr 29, 2015 at 4:24 PM, Martin Morgan <mtmorgan at fredhutch.org>
wrote:
> On 04/28/2015 01:04 PM, G?bor Cs?rdi 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()'.
>>
>
> stats::density() is an S3 generic, so igraph would define an S3 method,
> right? And in general a developer would avoid masking a function in a base
> package, so as not to require the user to distinguish between
> stats::density() and igraph::density(). Maybe the example is not meant
> literally.
>
Yes, maybe this is not the best example. I would not go into the details
here, because they are not really important for the current discussion.

> Being able to easily flag non-imported, non-base symbols would definitely
> improve the robustness of package code, even if not helping the end user
> disambiguate duplicate symbols.
>
In 'non-base' I assume you mean the single base package.

Just to address the second part, I think the end-user does not have to deal
with this. (Unless it is buggy like it is now.) The fact that ggplot2 calls
density() is not the user's concern.

Gabor

[...]

	[[alternative HTML version deleted]]

> And in general a developer would avoid masking a function
> in a base package, so as not to require the user to distinguish
> between stats::density() and igraph::density(). Maybe the
> example is not meant literally.
The 'filter' function in the popular 'dplyr' package masks the
one
that has been in the stats package forever, and they have nothing
in common, so that may give you an example.

Bill Dunlap
TIBCO Software
wdunlap tibco.com

On Wed, Apr 29, 2015 at 1:24 PM, Martin Morgan <mtmorgan at fredhutch.org>
wrote:
> On 04/28/2015 01:04 PM, G?bor Cs?rdi wrote:
>
>> When a symbol in a package is resolved, R looks into the package's
>> environment, and then into the package's imports environment. Then,
if the
>> symbol is still not resolved, it looks into the base package. So far so
>> good.
>>
>> If still not found, it follows the 'search()' path, starting
with the
>> global environment and then all attached packages, finishing with base
and
>> recommended packages.
>>
>> This can be a problem if a package uses a function from a base package,
>> but
>> it does not formally import it via the NAMESPACE file. If another
package
>> on the search path also defines a function with the same name, then
this
>> second function will be called.
>>
>> 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()'.
>>
>
> stats::density() is an S3 generic, so igraph would define an S3 method,
> right? And in general a developer would avoid masking a function in a base
> package, so as not to require the user to distinguish between
> stats::density() and igraph::density(). Maybe the example is not meant
> literally.
>
> Being able to easily flag non-imported, non-base symbols would definitely
> improve the robustness of package code, even if not helping the end user
> disambiguate duplicate symbols.
>
> Martin Morgan
>
>
>> I think that for a better solution, either
>> 1) the search path should not be used at all to resolve symbols in
>> packages, or
>> 2) only base packages should be searched.
>>
>> I realize that this is something that is not easy to change, especially
1)
>> would break a lot of packages. But maybe at least 'R CMD check'
could
>> report these cases. Currently it reports missing imports for non-base
>> packages only. Is it reasonable to have a NOTE for missing imports from
>> base packages as well?
>>
>> [As usual, please fix me if I am missing or misunderstood something.]
>>
>> Thank you, Best,
>> Gabor
>>
>>         [[alternative HTML version deleted]]
>>
>> ______________________________________________
>> R-devel at r-project.org mailing list
>> https://stat.ethz.ch/mailman/listinfo/r-devel
>>
>>
>
> --
> Computational Biology / Fred Hutchinson Cancer Research Center
> 1100 Fairview Ave. N.
> PO Box 19024 Seattle, WA 98109
>
> Location: Arnold Building M1 B861
> Phone: (206) 667-2793
>
>
> ______________________________________________
> R-devel at r-project.org mailing list
> https://stat.ethz.ch/mailman/listinfo/r-devel
>
	[[alternative HTML version deleted]]