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
