-
Notifications
You must be signed in to change notification settings - Fork 1
Iss028: Add @return and @example to every function with @export #32
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
Conversation
jhseeman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for doing this! One general style comment: there are places throughout where we have local objects clashing with global package names (ex: roadmap <- roadmap()). I think the examples will be clearer if we avoid this behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for adding this!
NAMESPACE
Outdated
| S3method(print,visit_sequence) | ||
| export("%>%") | ||
| export(.calc_bandwidths) | ||
| export(.construct_workflows) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can probably remove this export
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed!
| #' | ||
| #' @examples | ||
| #' | ||
| #' noise() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be pretty easy to add self-contained examples here, yes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added!
| #' | ||
| #' @examples | ||
| #' | ||
| #' start_method(start_func = start_resample) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should add some example arguments to be passed to start_resample
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I add a couple of examples!
R/invert.R
Outdated
| #' @param predictions A data frame with .pred | ||
| #' @param ... Other arguments | ||
| #' | ||
| #' @return A tibble with inverted model-generated values |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are we intending two different @return statements here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch. Updated!
| command. | ||
| # Or the development version from GitHub: | ||
| # install.packages("pak") | ||
| pak::pak("UrbanInstitute/tidysynthesis") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there a reason we're switching from install_github to pak?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm emulating tidymodels, which now uses pak: https://github.com/tidymodels/tidymodels
|
I responded to your feedback. I switched |
jhseeman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved
@exportfrom a few functions