roxygen2 icon indicating copy to clipboard operation
roxygen2 copied to clipboard

`@export` all.equal S3 method error

Open t-kalinowski opened this issue 1 year ago • 5 comments

Having an entry like this:

#' @export
all.equal.keras.src.backend.common.variables.KerasVariable <-
  function(target, current, ...) { }

Produces this in the NAMESPACE

S3method(all,equal.keras.src.backend.common.variables.KerasVariable)

It should match all.equal, not all.

Seems like a return of https://github.com/r-lib/roxygen2/issues/147

t-kalinowski avatar Feb 01 '24 15:02 t-kalinowski

Easy workaround with #' @exportS3Method base::all.equal

t-kalinowski avatar Feb 01 '24 15:02 t-kalinowski

all() is a (group) S3 generic, so the NAMESPACE declaration that ‘roxygen2’ generates is correct, even if it’s not the one you want/expect: ‘roxygen2’ has no way of knowing whether you want to define a method for the all.equal() or all() generic. Both are feasible. — You need to use @exportS3Method here to disambiguate, and you should generally do this for all generics that include dots, because they are all potentially ambiguous.

klmr avatar Apr 22 '24 11:04 klmr

Perhaps roxygen2 should emit a warning in this case, asking users to resolve the ambiguity?

t-kalinowski avatar Apr 22 '24 12:04 t-kalinowski

I still don't understand how to fix the Rd entry in this case, I can only seem to get

\method{all}{equal.integer64}(

to show up despite the NAMESPACE entry being S3method(all.equal,integer64)...

MichaelChirico avatar Oct 13 '24 20:10 MichaelChirico

I struggled for a while to see the @method workaround linked in the first comment.

Looking around for how other packages handle this I immediately find another CRAN package ({dbnR}) that's published with the same error:

https://github.com/cran/dbnR/blob/59d53badcf1fd67898c8254f39a9262633a715ed/man/all.equal.dbn.fit.Rd

\usage{
\method{all}{equal.dbn.fit}(target, current, ...)
}

Apparently there are 8 such packages:

https://github.com/search?q=org%3Acran+path%3A.Rd+%2F%5B%5C%5C%5Dmethod%5B%7B%5Dall%5B%7D%5D%5B%7B%5Dequal%5B.%5D%2F&type=code

So it looks like a complete resolution here requires

#' @method all.equal integer64
#' @exportS3Method all.equal integer64

Is that right? It seems like we could also at least reduce the redundancy here...

MichaelChirico avatar Oct 13 '24 20:10 MichaelChirico