Skip to content

Commit

Permalink
selector update (#130)
Browse files Browse the repository at this point in the history
**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?

![image](https://github.com/insightsengineering/cards/assets/26774684/2f8abc40-2620-431d-aee7-f94b32b3d53e)


**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".
  • Loading branch information
ddsjoberg authored Jan 31, 2024
1 parent a23f679 commit deb67f2
Show file tree
Hide file tree
Showing 7 changed files with 31 additions and 30 deletions.
2 changes: 1 addition & 1 deletion R/check_pkg_installed.R
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ get_pkg_dependencies <- function(reference_pkg = "cards") {
if (is.null(reference_pkg)) {
return(NULL)
}
description <- utils::packageDescription(reference_pkg)
description <- utils::packageDescription(reference_pkg) |> suppressWarnings()
if (identical(description, NA)) {
return(NULL)
}
Expand Down
4 changes: 2 additions & 2 deletions R/print.R
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ print.card <- function(x, n = NULL, columns = c("auto", "all"), n_col = 6L, ...)
dplyr::mutate(
across(
c(
all_ard_groups(FALSE, TRUE),
all_ard_variables(FALSE, TRUE),
all_ard_groups("levels"),
all_ard_variables("levels"),
any_of(c("context", "stat_label", "warning", "error"))
),
function(x) {
Expand Down
2 changes: 1 addition & 1 deletion R/print_ard_conditions.R
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ print_ard_conditions <- function(x, call = NULL) {
dplyr::tibble(
# this column is the messaging for which groups/variable the message appears in
cli_variable_msg =
dplyr::select(.y, all_ard_variables(levels = FALSE)) |>
dplyr::select(.y, all_ard_variables("names")) |>
dplyr::mutate(across(where(is.list), unlist)) |>
dplyr::slice(1L) |>
as.list() |>
Expand Down
27 changes: 15 additions & 12 deletions R/selectors.R
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,9 @@
#' as `dplyr::select()`. Function selects variables columns, e.g. columns
#' named `"variable"` or `"variable_level"`.
#'
#' @param variables logical indicating whether to select the columns with the
#' variable names. Default is `TRUE`
#' @param levels logical indicating whether to select the columns with the
#' variable levels. Default is `TRUE`
#' @param types (`character`)\cr
#' type of column to select. `"names"` selects the columns variable name columns,
#' and `"levels"` selects the level columns. Default is `c("names", "levels")`
#' @return tidyselect output
#' @name selectors
#'
Expand All @@ -26,28 +25,32 @@ NULL

#' @export
#' @rdname selectors
all_ard_groups <- function(variables = TRUE, levels = TRUE) {
if (isTRUE(variables) && isTRUE(levels)) {
all_ard_groups <- function(types = c("names", "levels")) {
types <- arg_match(types, values = c("names", "levels"), multiple = TRUE)

if (setequal(types, c("names", "levels"))) {
return(dplyr::matches("^group[0-9]+$|^group[0-9]+_level$"))
}
if (isTRUE(variables)) {
if (setequal(types, "names")) {
return(dplyr::matches("^group[0-9]+$$"))
}
if (isTRUE(levels)) {
if (setequal(types, "levels")) {
return(dplyr::matches("^group[0-9]+_level$"))
}
}

#' @export
#' @rdname selectors
all_ard_variables <- function(variables = TRUE, levels = TRUE) {
if (isTRUE(variables) && isTRUE(levels)) {
all_ard_variables <- function(types = c("names", "levels")) {
types <- arg_match(types, values = c("names", "levels"), multiple = TRUE)

if (setequal(types, c("names", "levels"))) {
return(dplyr::any_of(c("variable", "variable_level")))
}
if (isTRUE(variables)) {
if (setequal(types, "names")) {
return(dplyr::any_of("variable"))
}
if (isTRUE(levels)) {
if (setequal(types, "levels")) {
return(dplyr::any_of("variable_level"))
}
}
6 changes: 3 additions & 3 deletions R/tidy_ard_order.R
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@ tidy_ard_row_order <- function(x) {
# get columns that dictate ordering
dat <- x |>
dplyr::select(
all_ard_variables(variables = TRUE, levels = FALSE),
all_ard_groups(variables = TRUE, levels = FALSE),
all_ard_groups(variables = FALSE, levels = TRUE)
all_ard_variables("names"),
all_ard_groups("names"),
all_ard_groups("levels")
)

cols <- dat |>
Expand Down
12 changes: 5 additions & 7 deletions man/selectors.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions tests/testthat/test-selectors.R
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@ test_that("selectors work", {
)
expect_equal(
ard_testing |>
dplyr::select(all_ard_groups(TRUE, FALSE)) |>
dplyr::select(all_ard_groups("names")) |>
names(),
"group1"
)
expect_equal(
ard_testing |>
dplyr::select(all_ard_groups(FALSE, TRUE)) |>
dplyr::select(all_ard_groups("levels")) |>
names(),
"group1_level"
)
Expand All @@ -28,13 +28,13 @@ test_that("selectors work", {
)
expect_equal(
ard_testing |>
dplyr::select(all_ard_variables(TRUE, FALSE)) |>
dplyr::select(all_ard_variables("names")) |>
names(),
"variable"
)
expect_equal(
ard_testing |>
dplyr::select(all_ard_variables(FALSE, TRUE)) |>
dplyr::select(all_ard_variables("levels")) |>
names(),
"variable_level"
)
Expand Down

0 comments on commit deb67f2

Please sign in to comment.