Selector arg update #123
Assignees
Milestone
Comments
ddsjoberg
added a commit
that referenced
this issue
Jan 31, 2024
**What changes are proposed in this pull request?**
- updated the API for the `all_ard_*()` selecting functions. Previously,
there were two arguments that accepted TRUE or FALSE indicating whether
you wanted the columns of variable names and the columns of variable
levels. The vast majority of the time you want both, which is the
default. But when you begin to need one or the other, you need to
specify two arguments and if you don't name them, it was unclear which
columns you're selecting. In this update, we now only have one argument
that accepts `c("variables", "levels")`. Now when you need to select one
or the other, it also makes it easier to read and know which columns are
being selected.
@bzkrouse do you think the values would be more clear named something
like `"varnames"` and `"varlevels"`, something else?
I think the updated code is easier to grok?
**Reference GitHub issue associated with pull request.** _e.g., 'closes
#1'_
closes #123
--------------------------------------------------------------------------------
Reviewer Checklist (if item does not apply, mark is as complete)
- [ ] Ensure all package dependencies are installed:
`devtools::install_dev_deps()`
- [ ] PR branch has pulled the most recent updates from master branch:
`usethis::pr_merge_main()`
- [ ] If a bug was fixed, a unit test was added.
- [ ] Run `pkgdown::build_site()`. Check the R console for errors, and
review the rendered website.
- [ ] Code coverage is suitable for any new functions/features:
`devtools::test_coverage()`
- [ ] `usethis::use_spell_check()` runs with no spelling errors in
documentation
When the branch is ready to be merged:
- [ ] Update `NEWS.md` with the changes from this pull request under the
heading "`# cards (development version)`". If there is an issue
associated with the pull request, reference it in parentheses at the end
update (see `NEWS.md` for examples).
- [ ] Increment the version number using `usethis::use_version(which =
"dev")`
- [ ] Run `usethis::use_spell_check()` again
- [ ] Approve Pull Request
- [ ] Merge the PR. Please use "Squash and merge".
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
all_ard_groups(variables = TRUE, levels = TRUE)will becomeall_ard_groups("variables", "levels")and the same forall_ard_variable().This way it's easier to specify when you just need one or the other.
The text was updated successfully, but these errors were encountered: