Skip to content

Commit 18d635d

Browse files
fix factor to integer bug (#543)
**What changes are proposed in this pull request?** Fixed bug in `rename_ard_columns()` whereby factor variables were getting converted to integers and added param `fct_as_chr` as is used in `unlist_ard_columns()` (#542 , @alanahjonas95 ) Provide more detail here as needed. **Reference GitHub issue associated with pull request.** _e.g., 'closes #<issue number>'_ closes #542 -------------------------------------------------------------------------------- Pre-review Checklist (if item does not apply, mark is as complete) - [x] **All** GitHub Action workflows pass with a ✅ - [x] PR branch has pulled the most recent updates from master branch: `usethis::pr_merge_main()` - [x] If a bug was fixed, a unit test was added. - [x] Code coverage is suitable for any new functions/features (generally, 100% coverage for new code): `devtools::test_coverage()` - [x] Request a reviewer Reviewer Checklist (if item does not apply, mark is as complete) - [ ] 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()` 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). - [ ] **All** GitHub Action workflows pass with a ✅ - [ ] Approve Pull Request - [ ] Merge the PR. Please use "Squash and merge" or "Rebase and merge". _Optional Reverse Dependency Checks_: Install `checked` with `pak::pak("Genentech/checked")` or `pak::pak("checked")` ```shell # Check dev versions of `cardx`, `gtsummary`, and `tfrmt` which are in the `ddsjoberg` R Universe Rscript -e "options(checked.check_envvars = c(NOT_CRAN = TRUE)); checked::check_rev_deps(path = '.', n = parallel::detectCores() - 2L, repos = c('https://ddsjoberg.r-universe.dev', 'https://cloud.r-project.org'))" # Check CRAN reverse dependencies but run tests skipped on CRAN Rscript -e "options(checked.check_envvars = c(NOT_CRAN = TRUE)); checked::check_rev_deps(path = '.', n = parallel::detectCores() - 2, repos = 'https://cloud.r-project.org')" # Check CRAN reverse dependencies in a CRAN-like environment Rscript -e "options(checked.check_envvars = c(NOT_CRAN = FALSE), checked.check_build_args = '--as-cran'); checked::check_rev_deps(path = '.', n = parallel::detectCores() - 2, repos = 'https://cloud.r-project.org')" ``` --------- Co-authored-by: Becca Krouse <14199771+bzkrouse@users.noreply.github.com>
1 parent 06974ad commit 18d635d

File tree

4 files changed

+62
-2
lines changed

4 files changed

+62
-2
lines changed

NEWS.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
# cards (development version)
2+
3+
* Fixed bug in `rename_ard_columns()` whereby factor variables were getting converted to integers and added parameter `fct_as_chr` as is used in `unlist_ard_columns()` (#542)
4+
15
# cards 0.7.1.9007
26

37
* Adding `ard_tabulate_rows()` function to tabulate the number of rows in a data frame. (#531)

R/rename_ard_columns.R

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@
1616
#' where the `colname` element is available to inject into the string,
1717
#' e.g. `'Overall {colname}'` may resolve to `'Overall AGE'` for an AGE column.
1818
#' Default is `'{colname}'`.
19+
#' @param fct_as_chr (scalar `logical`)\cr
20+
#' When `TRUE`, factor elements will be converted to character before unlisting.
21+
#' When the column being unlisted contains mixed types of classes, the
22+
#' factor elements are often converted to the underlying integer value instead
23+
#' of retaining the label. Default is `TRUE`.
1924
#' @param unlist `r lifecycle::badge("deprecated")`
2025
#'
2126
#' @return data frame
@@ -37,7 +42,9 @@
3742
#' unlist_ard_columns()
3843
rename_ard_columns <- function(x,
3944
columns = c(all_ard_groups("names"), all_ard_variables("names")),
40-
fill = "{colname}", unlist = NULL) {
45+
fill = "{colname}",
46+
fct_as_chr = TRUE,
47+
unlist = NULL) {
4148
# check inputs ---------------------------------------------------------------
4249
if (!missing(unlist)) {
4350
lifecycle::deprecate_warn(
@@ -52,6 +59,8 @@ rename_ard_columns <- function(x,
5259
check_class(x, "card")
5360
process_selectors(x, columns = {{ columns }})
5461
check_scalar(fill)
62+
check_scalar_logical(fct_as_chr)
63+
5564
if (!is_empty(setdiff(columns, dplyr::select(x, all_ard_groups("names"), all_ard_variables("names")) |> names()))) {
5665
bad_columns <-
5766
setdiff(columns, dplyr::select(x, all_ard_groups("names"), all_ard_variables("names")) |> names())
@@ -125,6 +134,13 @@ rename_ard_columns <- function(x,
125134
dplyr::select(-"...ard_row_order...") |>
126135
dplyr::mutate(
127136
# replace NULL values with NA, then unlist
128-
across(all_of(all_new_names), ~ map(., \(value) value %||% NA) |> unlist())
137+
across(
138+
all_of(all_new_names),
139+
~ map(., \(value){
140+
if (isTRUE(fct_as_chr) && inherits(value, "factor")) value <- as.character(value)
141+
value %||% NA
142+
}) |>
143+
unlist()
144+
)
129145
)
130146
}

man/rename_ard_columns.Rd

Lines changed: 7 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

tests/testthat/test-rename_ard_columns.R

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,3 +53,36 @@ test_that("rename_ard_columns(unlist) lifecycle", {
5353
rename_ard_columns(unlist = "stat")
5454
)
5555
})
56+
57+
58+
test_that("rename_ard_columns(fct_as_chr)", {
59+
adsl_ <- ADSL |>
60+
dplyr::mutate(
61+
RACE = factor(RACE)
62+
)
63+
64+
# check fct_to_chr = TRUE
65+
res <- ard_tabulate(
66+
data = adsl_,
67+
by = TRT01A,
68+
variables = c(RACE, ETHNIC)
69+
) |>
70+
rename_ard_columns()
71+
72+
# Check that 'race' is a character vector
73+
expect_type(res$RACE, "character")
74+
75+
# Check that it contains the actual level string, not an integer "1", "2", etc.
76+
expect_true("WHITE" %in% res$RACE)
77+
78+
# check fct_to_chr = FALSE
79+
res <- ard_tabulate(
80+
data = adsl_,
81+
by = TRT01A,
82+
variables = c(RACE, ETHNIC)
83+
) |>
84+
rename_ard_columns(fct_as_chr = FALSE)
85+
86+
# Check that 'race' is an interger vector
87+
expect_type(res$RACE, "integer")
88+
})

0 commit comments

Comments
 (0)