Just for the record, I did submit PR#15451: 'Bug 15451 - PATCH: namespaceImportFrom() not to warn when the identical object is imported twice' on 2013-09-10 https://bugs.r-project.org/bugzilla3/show_bug.cgi?id=15451
/Henrik On Mon, Sep 9, 2013 at 2:37 PM, Henrik Bengtsson <h...@biostat.ucsf.edu> wrote: > Any intelligent comments on this before I submit a proposal/patch via > bugs.r-project.org? > > /Henrik > > On Fri, Aug 30, 2013 at 12:47 PM, Henrik Bengtsson <h...@biostat.ucsf.edu> > wrote: >> On Fri, Aug 30, 2013 at 11:51 AM, Henrik Bengtsson <h...@biostat.ucsf.edu> >> wrote: >>> Hi. >>> >>> On Fri, Aug 30, 2013 at 6:58 AM, Paul Gilbert <pgilbert...@gmail.com> wrote: >>>> This is related to the recent thread on correct NAMESPACE approach when >>>> writing S3 methods. If your methods are S4 I think pkgB does not need to >>>> export the generic. Just export the method and everything works magically >>>> and your problem disappears. For S3 methods there seems to be the >>>> difficultly you describe. Of course, the difference between S3 and S4 on >>>> this appears somewhat bug like. (I have not tested all this very carefully >>>> so I may have something wrong.) >>> >>> For the record, you're referring to R-devel thread 'Correct NAMESPACE >>> approach when writing an S3 method for a generic in another package' >>> started on Aug 24, 2013 >>> [https://stat.ethz.ch/pipermail/r-devel/2013-August/067221.html]. >>> Yes, it's closely related or possibly the same issue. However, I do >>> not find it odd that the S3 generic function needs to be exported >>> from/available via the namespace. Hence it needs to be export():ed by >>> at least one package/namespace. >>> >>> The real issue is when two package needs to export a generic function >>> with the same name, e.g. foo(). If the two generic functions are >>> defined differently (e.g. different arguments/signatures), they will >>> be in conflict with each other. If they are identical, this should >>> not be a problem, but here I might be wrong. However, there is also >>> the special case where one package reexports the generic function from >>> another package, e.g. PkgB::foo() exports PkgA:foo(). In this case, >>> the object 'foo' does actually not existing in the name space of >>> package PkgB - instead it provides a "redirect" to it, e.g. >>> >>>> PkgA::foo >>> function (...) >>> UseMethod("foo") >>> <environment: namespace:PkgA> >>> >>>> PkgB::foo >>> function (...) >>> UseMethod("foo") >>> <environment: namespace:PkgA> >>> >>>> exists("foo", envir=getNamespace("PkgB"), inherits=FALSE) >>> [1] FALSE >>> >>>> exists("foo", envir=getNamespace("PkgB"), inherits=TRUE) >>> [1] TRUE >>> >>>> identical(PkgB::foo, PkgA::foo) >>> [1] TRUE >>> >>> >>> The warning on "replacing previous import by 'PkgA::foo' when loading >>> 'PkgC'" that occurs due to import(PkgA, PkgB) is generated in >>> base::namespaceImportFrom() >>> [http://svn.r-project.org/R/trunk/src/library/base/R/namespace.R], >>> simply because it detects that "foo" (=n) has already been imported to >>> PkgC' namespace (=impenv): >>> >>> if (exists(n, envir = impenv, inherits = FALSE)) { >>> if (.isMethodsDispatchOn() && methods:::isGeneric(n, ns)) { >>> ## warn only if generic overwrites a function which >>> ## it was not derived from >>> ... >>> } >>> warning(sprintf(msg, sQuote(paste(nsname, n, sep = "::")), >>> sQuote(from)), call. = FALSE, domain = NA) >>> } >>> >>> Note how there is already code for avoiding "false" warnings on S4 >>> generic function. This is what we'd like to have also for S3 generic >>> functions, but it's much harder to test for such since they're not >>> formally defined. However, I'd argue that it is safe to skip the >>> warning *when the variable to be imported does not actually exist in >>> the package being imported* (e.g. when just rexported), i.e. >>> >>>>svn diff namespace.R >>> Index: namespace.R >>> =================================================================== >>> --- namespace.R (revision 63776) >>> +++ namespace.R (working copy) >>> @@ -871,6 +871,10 @@ >>> } >>> for (n in impnames) >>> if (exists(n, envir = impenv, inherits = FALSE)) { >>> + ## warn only if imported variable actually exists in the >>> + ## namespace imported from, which is not the case if >>> + ## the variable is rexported from another namespace >>> + if (!exists(n, envir = ns, inherits = FALSE)) next >>> if (.isMethodsDispatchOn() && methods:::isGeneric(n, ns)) { >>> ## warn only if generic overwrites a function which >>> ## it was not derived from >> >> Ok, if import(PkgA, PkgB) and PkgB reexports a *different* foo() than >> PkgA::foo(), say PkgZ::foo so identical(PkgB::foo, PkgA::foo) is >> FALSE, then there is indeed a conflict. An alternative patch: >> >>>svn diff namespace.R >> Index: namespace.R >> =================================================================== >> --- namespace.R (revision 63776) >> +++ namespace.R (working copy) >> @@ -871,6 +871,11 @@ >> } >> for (n in impnames) >> if (exists(n, envir = impenv, inherits = FALSE)) { >> + ## warn only if imported variable is non-identical to >> + ## the one already imported >> + getImp <- get(n, envir = impenv) >> + obj <- get(n, envir = ns) >> + if (identical(obj, getImp)) next >> if (.isMethodsDispatchOn() && methods:::isGeneric(n, ns)) { >> ## warn only if generic overwrites a function which >> ## it was not derived from >> >> /Henrik >> >>> >>> I'm planning to propose ("wishlist / enhancement"; it may even be a >>> bug) this over at https://bugs.r-project.org/. >>> >>> Comments, anyone? >>> >>> /Henrik >>> >>> >>>> Paul >>>> >>>> Henrik Bengtsson <h...@biostat.ucsf.edu> wrote: >>>> >>>>>Hi, >>>>> >>>>>SETUP: >>>>>Consider three packages PkgA, PkgB and PkgC. >>>>> >>>>>PkgA defines a generic function foo() and exports it; >>>>> >>>>>export(foo) >>>>> >>>>>PkgB imports PkgA::foo() and re-exports it; >>>>> >>>>>importFrom(PkgA, foo) >>>>>export(foo) >>>>> >>>>>PkgC imports everything from PkgA and PkgB: >>>>> >>>>>imports(PkgA, PkgB) >>>>> >>>>> >>>>>PROBLEM: >>>>>Loading or attaching the namespace of PkgC will generate a warning: >>>>> >>>>> replacing previous import by 'PkgA::foo' when loading 'PkgC' >>>>> >>>>>This in turn causes 'R CMD check' on PkgC to generate a WARNING (no-go at >>>>>CRAN): >>>>> >>>>>* checking whether package 'PkgC' can be installed ... WARNING >>>>>Found the following significant warnings: >>>>> Warning: replacing previous import by 'PkgA::foo' when loading >>>>>'CellularAutomaton' >>>>> >>>>> >>>>>FALSE? >>>>>Isn't it valid to argue that this is a "false" warning, because >>>>>identical(PkgB::foo, PkgA::foo) is TRUE and therefore has no effect? >>>>> >>>>> >>>>>/Henrik >>>>> >>>>>PS. The above can be avoided by using explicit importFrom() on PkgA >>>>>and PkgB, but that's really tedious. In my case this is out of my >>>>>reach, because I'm the author of PkgA and PkgB but not many of the >>>>>PkgC packages. >>>>> >>>>>______________________________________________ >>>>>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
