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
