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

@export all.equal S3 method error #1587

Open
t-kalinowski opened this issue Feb 1, 2024 · 5 comments
Open

@export all.equal S3 method error #1587

t-kalinowski opened this issue Feb 1, 2024 · 5 comments

Comments

@t-kalinowski
Copy link
Member

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 #147

@t-kalinowski
Copy link
Member Author

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

@klmr
Copy link
Contributor

klmr commented Apr 22, 2024

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.

@t-kalinowski
Copy link
Member Author

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

@MichaelChirico

This comment was marked as resolved.

@MichaelChirico
Copy link
Contributor

MichaelChirico commented Oct 13, 2024

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...

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