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

Resolve "... may be used in an incorrect context" check warnings #465

Closed
brookslogan opened this issue Jun 18, 2024 · 3 comments · Fixed by #468
Closed

Resolve "... may be used in an incorrect context" check warnings #465

brookslogan opened this issue Jun 18, 2024 · 3 comments · Fixed by #468

Comments

@brookslogan
Copy link
Contributor

brookslogan commented Jun 18, 2024

This appears to be causing checks to be marked as failures. We do this very deliberately here (and probably in an analogous spot for epix_slides); if f is missing then ..1 is a tidy computation, not an argument to the computation. This makes it happen.

Some possible resolutions:

  • Pass around nonsense: make the conversion of ..1 to a function (within as_slide_computation) take in dots and ignore them, and remove the ... <- missing_arg() lines.
  • Don't pass ... to f: either
    • Disallow using ... for arg passing, or
    • Make as_slide_computation bake any args into f if f is a function, rather than use f as-is (something like return(function(x,gk,rtv) f(x,gk,rtv,...)), and no longer pass ... to computations within the group_modify computations of epi[x]_slide. For efficiency's sake, only do this if there are a nonzero number of dots.
    • [^ potentially like the above, but instead of baking args into `f`, have `as_slide_computation()` output both an `f` and a `group_modify_baked()` function that bakes in the dots if appropriate, and use that instead of `group_modify()`. Perhaps it could actually implement a `baker()` function factory to bake args into whatever function we want, but implementing that might be a bit gnarlier.]

@nmdefries do these sound like solutions? Any preferences?

@nmdefries
Copy link
Contributor

Hm, we've had this warning for a while. I didn't think it was causing CI failures.

What about assign("...", missing_arg())?

@brookslogan
Copy link
Contributor Author

That's what I thought, but I didn't originally see any other warnings and Ctrl+F didn't seem to work. Perhaps the 2 warnings actually were actually "checking for code/documentation mismatches ... WARNING" and "checking Rd \usage sections ... WARNING" from not running document, rather than " Warning: : ... may be used in an incorrect context (/home/runner/work/epiprocess/epiprocess/check/epiprocess.Rcheck/00_pkg_src/epiprocess/R/slide.R:307)" and "Warning: : ... may be used in an incorrect context (/home/runner/work/epiprocess/epiprocess/check/epiprocess.Rcheck/00_pkg_src/epiprocess/R/grouped_epi_archive.R:320)". CI auto-document didn't trigger a re-check. Let me try to force one.

assign seems like a good hack to test out.

@brookslogan
Copy link
Contributor Author

Okay, looks like it was the lack of document() causing the warnings that actually counted toward that check error. This is lower priority but probably good to get rid of some check warning spam and any future confusion like mine above. I'm not 100% sure assign does the equivalent things so we'll probably need to test it.

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