Skip to content

Clean up epi_slide f parameter documentation #143

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

Closed
rachlobay opened this issue Jul 13, 2022 · 1 comment · Fixed by #144 or #538
Closed

Clean up epi_slide f parameter documentation #143

rachlobay opened this issue Jul 13, 2022 · 1 comment · Fixed by #144 or #538
Assignees

Comments

@rachlobay
Copy link
Collaborator

As discussed with Logan, update the @param documentation for the f parameter in epi_slide. The following are the points to address:

  • Instead of “If a function, f must take x, a data frame with the same column names as the original object; followed by any number of named arguments; and ending with ...“, the proper thing may be that f should take x, g/k/key/think-of-a-better-name, and named args (dots don’t seem necessary; I think originally they were just there to absorb the unlisted g).
  • Correct "same column names as the original object" --- we might only get the non-grouping columns here. [This raises a separate issue. We are feeding the user an epi_df via f that might not have the geo_value column as one might expect. Hopefully we can get away with it, but maybe this needs to be a proper epi_df or just a tibble instead.]
  • If you think it clarifies things, change “named arguments” -> “named arguments (which will be taken from ...)“.
  • Make sure that the description of g above matches/resembles how ?group_modify describes such a parameter, if it makes sense.
  • In the docs for the f being a formula case, describe .y in the same manner as g, if .y can indeed be used [I'm guessing that it is usable].

Originally mentioned in PR #85.

@rachlobay rachlobay self-assigned this Jul 13, 2022
@rachlobay rachlobay linked a pull request Jul 16, 2022 that will close this issue
@brookslogan
Copy link
Contributor

brookslogan commented Nov 17, 2022

Still see remnants of the old docs somehow. [In slide.Rmd rather than slide.R. I must have used these slide.Rmd text as the basis for another slide.R doc update causing a reversion on a feature branch.]

@brookslogan brookslogan reopened this Nov 17, 2022
@brookslogan brookslogan assigned brookslogan and unassigned rachlobay Nov 17, 2022
@dshemetov dshemetov self-assigned this Oct 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants