R CMD CHECK WARNING inappropriate for S4 method? - R-devel

Sun, Jun 8, 2008 3:04 PM #

The package 'graph' defines classes graph and graphNEL (extending
graph) and a union,graph,graph-method. These are all exported and
fully documented.

The package 'GSEABase' Imports: graph and importClassesFrom(graph,
graphNEL). GSEABase defines methods on union for its own classes (not
graph / graphNEL), and has exportMethods(union).
union,graph,graph-method is not used in GSEABase.

R version 2.8.0 Under development (unstable) (2008-06-08 r45879)
reports from R CMD CHECK GSEABase

* checking for missing documentation entries ... WARNING
Undocumented S4 methods:
  generic 'union' and siglist 'graph,graph'

I guess union,graph,graph-method is exposed to the user indirectly by
its import along with the graph class and export along with the
newly-defined union methods.

It would seem inappropriate to document union,graph,graph-method in
GSEABase. Making GSEABase Depend: graph rather than Import: graph does
not help.

Any guidance welcome.

Thanks,

Martin

Martin Morgan
Computational Biology / Fred Hutchinson Cancer Research Center
1100 Fairview Ave. N.
PO Box 19024 Seattle, WA 98109

Location: Arnold Building M2 B169
Phone: (206) 667-2793

John Chambers

Mon, Jun 9, 2008 11:35 AM #

This one is subtle, and has mostly to do with namespace and imports, 
only secondarily to do with methods.  Bear with a fairly long story.

The description below is for a simple package P1 that imitates the 
pattern of imports in GSEABase.

First, what's generating the warning?  The function undoc() in the tools 
package looks for all the methods defined in the current package, for 
"union" in this case.  In r-devel, that uses the function findMethods().

What it gets is:
 > names(findMethods("union", where = 2))
[1] "ANY#ANY"             "graph#graph"         "testClass#testClass"

(where "testClass" is the new class that P1 is adding.)

What it "normally" gets in these circumstances is:
 > names(findMethods("union", where = 2))
[1] "testClass#testClass"

In the first case, undoc() naturally assumes you created all the methods 
and wants you to document them.

The question, then, is why are you getting the extra methods?

The answer, to the extent I understand it, is that P1 (and GSEABase) 
used the
  Imports:graph
directive in the DESCRIPTION file, instead of explicitly doing the 
import() commands in the NAMESPACE file.  Just what this does 
differently  I'm not sure, but the effect is that the generic version of 
union() in the "graph" package is no longer in the imports environment 
for package P1:

 > n1 = asNamespace("P1")
 > objects(parent.env(n1), all=TRUE, pattern="union")
character(0)

The result is that P1 owns the union() generic and all its methods.  In 
particular, a generic function "union" is created in P1.

The alternative way to specify the imports is to use the NAMESPACE file, 
say with:

import(graph)

Package "graph" must be added to the Depends: line of the DESCRIPTION 
file, instead of being in the Imports: line.  (Otherwise, you still get 
the new generic, owning all the methods.)

Now the generic version of union() is in the imports environment for P1; 
the setMethod() call in P1 does not create a new generic.  We can see 
this by:
 > n1 = asNamespace("P1")
 > objects(parent.env(n1), all=TRUE, pattern="union")
[1] ".__M__union:base" ".__T__union:base" "union"          


One final catch: In this version, you do need to ensure that the user's 
call to union() sees the generic version at run time, and not the 
nongeneric version in base.  Either require(graph) or, in P1's NAMESPACE:

export(union)

Perhaps needless to say, this is all a bit more subtle than one would like.

John

Martin Morgan wrote:

Martin Morgan

Mon, Jun 9, 2008 12:51 PM #

Thank you for the assistance.

I moved graph from Imports to Depends (arguably appropriate anyway, as
the reason for graph is that GSEABase defines setAs(..., "graphNEL"))
added importFrom(graph, union) to NAMESPACE, and removed
setGeneric("union", ...) in GSEABase; union was already being exported
from GSEABase.

For information, this has the consequence of invalidating what I
thought was 'careful' code invoking GSEABase::union(), since the union
I want is now graph::union (moderately inexplicable to users who
though they were dealing with gene sets and not graphs; and different
from GSEABase::intersect, which graph does not provide a generic
for). Mostly the user will never know, as unqualified 'union' or
'intersect' work appropriately.

Thanks again

Martin

John Chambers <jmc at r-project.org> writes:

This one is subtle, and has mostly to do with namespace and imports,
only secondarily to do with methods.  Bear with a fairly long story.

The description below is for a simple package P1 that imitates the
pattern of imports in GSEABase.

First, what's generating the warning?  The function undoc() in the
tools package looks for all the methods defined in the current
package, for "union" in this case.  In r-devel, that uses the function
findMethods().

What it gets is:

 > names(findMethods("union", where = 2))

[1] "ANY#ANY"             "graph#graph"         "testClass#testClass"

(where "testClass" is the new class that P1 is adding.)

What it "normally" gets in these circumstances is:

 > names(findMethods("union", where = 2))

[1] "testClass#testClass"

In the first case, undoc() naturally assumes you created all the
methods and wants you to document them.

The question, then, is why are you getting the extra methods?

The answer, to the extent I understand it, is that P1 (and GSEABase)
used the
  Imports:graph
directive in the DESCRIPTION file, instead of explicitly doing the
import() commands in the NAMESPACE file.  Just what this does
differently  I'm not sure, but the effect is that the generic version
of union() in the "graph" package is no longer in the imports
environment for package P1:

 > n1 = asNamespace("P1")
 > objects(parent.env(n1), all=TRUE, pattern="union")

character(0)

The result is that P1 owns the union() generic and all its methods.
In particular, a generic function "union" is created in P1.

The alternative way to specify the imports is to use the NAMESPACE
file, say with:

import(graph)

Package "graph" must be added to the Depends: line of the DESCRIPTION
file, instead of being in the Imports: line.  (Otherwise, you still
get the new generic, owning all the methods.)

Now the generic version of union() is in the imports environment for
P1; the setMethod() call in P1 does not create a new generic.  We can
see this by:

 > n1 = asNamespace("P1")
 > objects(parent.env(n1), all=TRUE, pattern="union")

[1] ".__M__union:base" ".__T__union:base" "union"          


One final catch: In this version, you do need to ensure that the
user's call to union() sees the generic version at run time, and not
the nongeneric version in base.  Either require(graph) or, in P1's
NAMESPACE:

export(union)

Perhaps needless to say, this is all a bit more subtle than one would like.

John





Martin Morgan wrote:

The package 'graph' defines classes graph and graphNEL (extending
graph) and a union,graph,graph-method. These are all exported and
fully documented.

The package 'GSEABase' Imports: graph and importClassesFrom(graph,
graphNEL). GSEABase defines methods on union for its own classes (not
graph / graphNEL), and has exportMethods(union).
union,graph,graph-method is not used in GSEABase.

R version 2.8.0 Under development (unstable) (2008-06-08 r45879)
reports from R CMD CHECK GSEABase

* checking for missing documentation entries ... WARNING
Undocumented S4 methods:
  generic 'union' and siglist 'graph,graph'

I guess union,graph,graph-method is exposed to the user indirectly by
its import along with the graph class and export along with the
newly-defined union methods.

It would seem inappropriate to document union,graph,graph-method in
GSEABase. Making GSEABase Depend: graph rather than Import: graph does
not help.

Any guidance welcome.

Thanks,

Martin

Martin Morgan
Computational Biology / Fred Hutchinson Cancer Research Center
1100 Fairview Ave. N.
PO Box 19024 Seattle, WA 98109

Location: Arnold Building M2 B169
Phone: (206) 667-2793