Gábor Csárdi
2015-Apr-29 17:00 UTC
[Rd] R CMD check and missing imports from base packages
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]]
Gabriel Becker
2015-Apr-29 17:45 UTC
[Rd] R CMD check and missing imports from base packages
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)? Also, if your package Depends on another package, instead of importing it (e.g. because the end user will need functions in it in order to meaningfully use your functions), I imagine you *want* to hit symbols in that package before base, right? Otherwise the Depends mechanism becomes somewhat crippled because you'd need to import symbols from packages you Depend on, at least in certain cases. 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). 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? 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]]
Gábor Csárdi
2015-Apr-29 17:57 UTC
[Rd] 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]]