-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Collection of documentation tweaks #6352
Conversation
#' union_all(a, b) | ||
NULL | ||
|
||
#' @name setops |
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.
See r-lib/roxygen2#1408 as an approach to hopefully make this easier in the future.
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.
Looks great, just a bunch of small tweaks
#' - `NULL`: the default, returns the selected columns in a data frame | ||
#' without applying a transformation. This is useful for when you want to | ||
#' use a function that takes a data frame. |
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.
If we add pick()
, we should probably slowly deprecate this. So then there aren't two ways to do the same thing
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.
Agreed
#' it will error, and require you to specify the name. | ||
#' it will use `nn`. If there's a column called `n` and `nn`, it'll use | ||
#' `nnn`, and so on, adding `n`s until it gets a new name. |
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.
Do you want to mention that it outputs a message in this case?
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.
No 😄 I don't think this behaviour is that important (and I generally regret it as being too clever)
#' @aliases union | ||
#' @usage union(x, y, ...) | ||
#' @importFrom generics union | ||
#' @export union |
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.
_pkgdown.yml
Outdated
- across | ||
- c_across | ||
- between | ||
- c_across |
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.
Do you want across()
and c_across()
mentioned both here and in their special Multiple columns
section?
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.
Oops no
_pkgdown.yml
Outdated
- case_when | ||
- coalesce | ||
- cumany | ||
- desc | ||
- if_else | ||
- lead | ||
- order_by | ||
- "n" |
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.
The context dependent expressions that come from including n()
here somehow feel like they dont belong in this "Vector functions" section
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.
Agreed, moved to grouping since that's where they mostly live. Could also consider moving cur_column()
into across()
or its own file.
Co-authored-by: Davis Vaughan <davis@rstudio.com>
Fixes #6017. Fixes #5958. Fixes #6233. Fixes #5943. Fixes #6326. Fixes #6205.