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

length method vs R CMD Check #342

Closed
jonthegeek opened this issue Sep 10, 2023 · 7 comments · Fixed by #343
Closed

length method vs R CMD Check #342

jonthegeek opened this issue Sep 10, 2023 · 7 comments · Fixed by #343

Comments

@jonthegeek
Copy link
Contributor

I'm not sure how to reprex this, because so far I only see the issue inside R CMD Check. This is the warning I get:

❯ checking for code/documentation mismatches ... WARNING
Warning in formals(fun) : argument is not a function

I got it by trying to define a length method.

S7::method(length, rapid) <- function(x) length(x@info)

It otherwise works with the dev version of S7 (but fails with the CRAN version).

I'm assuming that it's somehow related to length being a primitive rather than an actual S3 generic, but beyond that I'm not sure where to look.

@hadley
Copy link
Member

hadley commented Sep 11, 2023

Would you mind providing a basic package reprex?

@jonthegeek
Copy link
Contributor Author

Sure, I'll put something together, including the failed check.

@jonthegeek
Copy link
Contributor Author

https://github.com/jonthegeek/S7primitivereprex/actions/runs/6149344761/job/16685015875?pr=1

Somewhat shockingly, I had to include the R/S7primitivereprex-package.R file (or possibly specifically the corresponding Rd) to get the error, specifically this piece:

#' @keywords internal
"_PACKAGE"

I hope that helps track it down!

@hadley
Copy link
Member

hadley commented Sep 11, 2023

You just need one documentation file to be present, which I guess isn't too surprising given that the error comes from the "checking for code/documentation mismatches" check.

@hadley
Copy link
Member

hadley commented Sep 11, 2023

I see the same problem when registering a method for mean(), so it doesn't appear to be just related to primitives.

You can see the problem more easily with tools::codoc("S7primitivereprex", ".")

A bit of rlang gymnastics yields a back trace for the warning:

    ▆
  1. ├─base::withCallingHandlers(...)
  2. ├─tools::codoc("S7primitivereprex", ".")
  3. │ └─base::lapply(functions_in_S3Table, function(f) formals(S3Table[[f]]))
  4. │   └─tools (local) FUN(X[[i]], ...)
  5. │     └─base::formals(S3Table[[f]])

And it's calling it with S3Table[[".S7_methods"]] which is added by external_methods_add().

hadley added a commit that referenced this issue Sep 11, 2023
This prevents it from being listed in `functions_in_S3_table` (https://github.com/wch/r-source/blob/16c4fbf48efb4d43ef5dccadc86576100edd85b4/src/library/tools/R/QC.R#L370-L383) and later generating a warning when calling `formals` on each element of that list.

Fixes #342
@hadley
Copy link
Member

hadley commented Sep 11, 2023

@jonthegeek thanks for trying S7 out and reporting these problems!

@jonthegeek
Copy link
Contributor Author

Happy to help! I'd been trying to decide how to structure a data--of-a-particular-shape-focused package, and S7 appeared on CRAN just in time 😊

hadley added a commit that referenced this issue Sep 12, 2023
)

This prevents it from being listed in `functions_in_S3_table` (https://github.com/wch/r-source/blob/16c4fbf48efb4d43ef5dccadc86576100edd85b4/src/library/tools/R/QC.R#L370-L383) and later generating a warning when calling `formals` on each element of that list.

Fixes #342
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

Successfully merging a pull request may close this issue.

2 participants