[Rd] S4 Generics and NAMESPACE : justified warning ?

[Yohan Chalabi]

Dear list,

It seems that a package (pkgB) using another package (pkgA) with S4
generics formed by taking existing functions (for example 'plot') must
not import the existing functions ('plot') in its namespace to avoid
the warning "replacing previous import: plot".

Suppose we use the simple 'import' directive in the name space of
pkgB.

pkgA and pkgB files would be

pkgA/NAMESPACE:
    import(graphics)
    import(methods)
    exportClasses("classA")
    exportMethods("plot")

pkgA/R/pkgA.R:
    setClass("classA", contains = "matrix")
    setMethod("plot", "classA", function(x, y, ...) NULL)

pkgB/NAMESPACE:
    import(methods)
    import(garphics)
    import(pkgA)

Loading pkgB would then generate a warning because the generic 'plot'
in pkgA overwrites the 'plot' function imported from graphics name
space :

Loading required package: pkgA
Warning message:
In namespaceImportFrom(self, asNamespace(ns)) :
  replacing previous import: plot

As far as I understood it, one must explicitly import the functions one
needs in pkgB with 'importFrom' to avoid the previous warning. In
our example, one would then take care to not import the 'plot'
function from garphics name space.

This makes the maintenance of packages using S4 packages rather
tedious because one needs to import exactly what one needs rather than
using the simple 'import' directive.

Moreover, the warning "replacing previous import:" is confusing. In this
case we have a generic replacing its existing function like
'setMethod' would do and not a function replacing another function
with the same name.

IMO it is normal that a function in a previous import can be replaced
by its generic. It should not create a warning. But a warning should
be generated when a generic is replaced by another generic.

Does this make sense ?

This could be implemented in 'namespaceImportFrom' with the patch given
bellow.

It would greatly help the maintanance of packages which use S4
packages, because one could use the simple 'import' directive rather
than 'importFrom'.

Looking forward for your comments.

Regards,
Yohan


#################################################################################

Index: src/library/base/R/namespace.R
===================================================================
--- src/library/base/R/namespace.R	(revision 49278)
+++ src/library/base/R/namespace.R	(working copy)
@@ -797,8 +797,12 @@
 	}
     }
     for (n in impnames)
-        if (exists(n, envir = impenv, inherits = FALSE))
-            warning(msg, " ", n)
+        if (exists(n, envir = impenv, inherits = FALSE)) {
+            if (.isMethodsDispatchOn() && methods:::isGeneric(n, ns)) {
+                if (methods:::isGeneric(n, impenv))
+                    warning("replacing previous generic import: ", n)
+            } else warning(msg, " ", n)
+        }
     importIntoEnv(impenv, impnames, ns, impvars)
     if (register) {
         addImports(self, ns,

#################################################################################


-- 
PhD student
Swiss Federal Institute of Technology
Zurich

www.ethz.ch