Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

S4 signature for trailing ANY is not created #460

Closed
wahani opened this issue Mar 6, 2016 · 4 comments
Closed

S4 signature for trailing ANY is not created #460

wahani opened this issue Mar 6, 2016 · 4 comments

Comments

@wahani
Copy link

wahani commented Mar 6, 2016

Hi, I have the following problem while documenting a S4-generic and its methods:

When I define a method where the trailing argument has type "ANY" an entry in the documentation is only created for the signature "numeric" instead of numeric,ANY in the following example:

#' new generic
#'
#' @rdname myGeneric
#' @export
setGeneric("myGeneric", function(x, y) standardGeneric("myGeneric"))

#' @rdname myGeneric
#' @export
setMethod("myGeneric", c("numeric", "ANY"), function(x, y) x)

This results in a warning from R CMD check because the numeric,ANY method documentation entry is missing. I suspect that this happens because methods("myGeneric") will report that there is only a method for signature "numeric" and hence roxygen will only create a doc entry for this signature. This is only an issue when the last argument is of type "ANY" - a different order will work - since methods will report the correct signature.

Don't know if this is solvable within roxygen2. My attempts to bypass the issue by creating the doc entry manually have failed so far. I am using roxygen2 version 5.0.1 and R3.2.2 on Ubuntu 14.10

Thanks,
-s

@hadley
Copy link
Member

hadley commented Aug 29, 2016

The basic problem is that the getMethod() doesn't seem to be returning that info:

library(methods)
setGeneric("foo", function(x, y) standardGeneric("foo"))
#> [1] "foo"
setMethod("foo", c("numeric", "ANY"), function(x, y) x)
#> [1] "foo"
methods::getMethod("foo", c("numeric", "ANY"))@target
#> An object of class "signature"
#>         x 
#> "numeric"

@lawremi any idea what might be going on here?

@wahani are you sure you can just do setMethod("myGeneric", c("numeric"), function(x, y) x) ?

@lawremi
Copy link

lawremi commented Aug 29, 2016

Trailing ANY's are dropped from the signature by matchSignature(). The behavior has existed from the initial checkin of the methods package in 2001. I guess one motivation was that people are often lazy when defining their generic signatures, so methods end up having an excessive list of ANY's printed as their signature. Once a method is defined without ANY for an argument, the ANY's are included for that argument in every method's signature. Obviously, this behavior is not ideal, but I'm hesitant to change it.

My guess is that Roxygen queries the method for its signature just after it is defined, and if every method so far has ANY for some argument, that argument is excluded from the signature. Later, a method is defined with a specific class for that argument, and thus all of the method signatures are updated.

Probably the most pragmatic way forward is for roxygen to do the equivalent of methods:::.matchSigLength() on the signature, except using length(generic@signature) instead of methods:::.getGenericSigLength() to get the signature length.

@hadley hadley closed this as completed in 848e5a3 Aug 29, 2016
@wahani
Copy link
Author

wahani commented Aug 29, 2016

Thank you for the clarification.

With the current version 3.3.1 I am not able to reproduce the warning. The behavior of roxygen is unchanged, however R CMD check seams to be satisfied with the given man page. I tested this with R CMD check --as-cran and devtools::check_man and can't find any problem.

@wahani
Copy link
Author

wahani commented Aug 29, 2016

I tested the thing with after the fix. I now get the correct entry in the Rd file: \alias{myGeneric,numeric,ANY-method} and the following warning in R CMD check

* checking for missing documentation entries ... WARNING
Undocumented S4 methods:
  generic 'myGeneric' and siglist 'numeric'
All user-level objects in a package (including S4 classes and methods)
should have documentation entries.
See chapter ‘Writing R documentation files’ in the ‘Writing R
Extensions’ manual.
* checking for code/documentation mismatches ... OK
* checking Rd \usage sections ... WARNING
Objects in \usage without \alias in documentation object 'myGeneric':
  ‘\S4method{myGeneric}{numeric}’

Functions with \usage entries need to have the appropriate \alias
entries, and all their arguments documented.
The \usage entries must correspond to syntactically valid R code.
See chapter ‘Writing R documentation files’ in the ‘Writing R
Extensions’ manual.

Basically the opposite from what I initially reported. Adding \alias{myGeneric,numeric-method} to the Rd file will resolve the warning.

hadley added a commit that referenced this issue Aug 29, 2016
This reverts commit 848e5a3.

Since it no longer seems to be necessary #460
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants