Skip to content
Prev 46567 / 63421 Next

"False" warning on "replacing previous import" when re-exporting identical object

Hi.
On Fri, Aug 30, 2013 at 6:58 AM, Paul Gilbert <pgilbert902 at gmail.com> wrote:
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.
function (...)
UseMethod("foo")
<environment: namespace:PkgA>
function (...)
UseMethod("foo")
<environment: namespace:PkgA>
[1] FALSE
[1] TRUE
[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.
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

I'm planning to propose ("wishlist / enhancement"; it may even be a
bug) this over at https://bugs.r-project.org/.

Comments, anyone?

/Henrik