From 49b95f7c2f28be8109ad32a43c66a0c41f81a0b3 Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Mon, 24 Jun 2024 20:42:47 +0000 Subject: [PATCH 01/11] raise attention --- R/add_stat.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/add_stat.R b/R/add_stat.R index 830d225b68..5fff22e5b9 100644 --- a/R/add_stat.R +++ b/R/add_stat.R @@ -147,7 +147,7 @@ add_stat <- function(x, fns, location = everything() ~ "label") { df_new_stat <- dplyr::tibble(variable = names(fns)) |> dplyr::mutate( - summary_type = map_chr(.data$variable, ~ x$inputs$type[[.x]]), + summary_type = map_chr(.data$variable, ~ x$inputs$type[[.x]]), # tbl_continuous objects generally don't have a "$inputs$type" object nested inside row_type = map_chr(.data$variable, ~ location[[.x]]), label = map2( .data$variable, .data$row_type, From 7402616e9f23e6d50543f776bb0042f96b61ab5f Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Mon, 24 Jun 2024 21:08:50 +0000 Subject: [PATCH 02/11] need at least this to get old tests working --- R/tbl_continuous.R | 2 ++ tests/testthat/test-add_stat.tbl_continuous.R | 28 +++++++++++++++++++ 2 files changed, 30 insertions(+) create mode 100644 tests/testthat/test-add_stat.tbl_continuous.R diff --git a/R/tbl_continuous.R b/R/tbl_continuous.R index 758f454ebb..4abf854aef 100644 --- a/R/tbl_continuous.R +++ b/R/tbl_continuous.R @@ -37,6 +37,7 @@ tbl_continuous <- function(data, variable, include = everything(), digits = NULL, + type = NULL, by = NULL, statistic = everything() ~ "{median} ({p25}, {p75})", label = NULL) { @@ -94,6 +95,7 @@ tbl_continuous <- function(data, digits = digits ) + type <- assign_summary_type(data, variables = include, value = NULL) # save processed function inputs --------------------------------------------- tbl_continuous_inputs <- as.list(environment()) diff --git a/tests/testthat/test-add_stat.tbl_continuous.R b/tests/testthat/test-add_stat.tbl_continuous.R new file mode 100644 index 0000000000..9f93e11501 --- /dev/null +++ b/tests/testthat/test-add_stat.tbl_continuous.R @@ -0,0 +1,28 @@ +test_that("add_stat for tbl_continuous() works", { + tt <- + trial %>% + tbl_continuous( + age, + include = grade, + by = trt + ) + + add_stat_test1 <- function(data, variable, by, ...) { + tibble::tibble(test_col = "Ugh") + } + + add_stat_test2 <- function(data, variable, by, ...) { + tibble::tibble(test_col = rep_len("Ugh", 3)) + } + + expect_error( + tt %>% + add_stat(fns = everything() ~ add_stat_test1), + NA + ) + expect_error( + tt %>% + add_stat(fns = everything() ~ add_stat_test2, location = everything() ~ "level"), + NA + ) +}) From 2c672f28f04cc5a2e553cc8b8db6d56b295b501e Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Mon, 24 Jun 2024 21:55:44 +0000 Subject: [PATCH 03/11] how to use include for formula formation? --- R/add_stat.R | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/R/add_stat.R b/R/add_stat.R index 5fff22e5b9..bd2141eb58 100644 --- a/R/add_stat.R +++ b/R/add_stat.R @@ -147,7 +147,9 @@ add_stat <- function(x, fns, location = everything() ~ "label") { df_new_stat <- dplyr::tibble(variable = names(fns)) |> dplyr::mutate( - summary_type = map_chr(.data$variable, ~ x$inputs$type[[.x]]), # tbl_continuous objects generally don't have a "$inputs$type" object nested inside + summary_type = map_chr(.data$variable, ~ x$inputs$type[[.x]]), + # tbl_continuous objects generally didn't have a "$inputs$type" object nested inside + # added it back to tbl_continuous row_type = map_chr(.data$variable, ~ location[[.x]]), label = map2( .data$variable, .data$row_type, @@ -231,6 +233,7 @@ eval_fn_safe <- function(variable, tbl, fn) { data = tbl$inputs$data, variable = variable, by = tbl$inputs$by, + include = tbl$inputs$include, tbl = tbl ) ) From bac05213e0b479ceac75d295d5baa2c517894992 Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Tue, 25 Jun 2024 15:53:08 +0000 Subject: [PATCH 04/11] should work --- R/tbl_continuous.R | 3 +- man/tbl_strata.Rd | 2 +- tests/testthat/test-add_stat.tbl_continuous.R | 48 +++++++++++++++---- 3 files changed, 40 insertions(+), 13 deletions(-) diff --git a/R/tbl_continuous.R b/R/tbl_continuous.R index 4abf854aef..e1029bc8c2 100644 --- a/R/tbl_continuous.R +++ b/R/tbl_continuous.R @@ -37,7 +37,6 @@ tbl_continuous <- function(data, variable, include = everything(), digits = NULL, - type = NULL, by = NULL, statistic = everything() ~ "{median} ({p25}, {p75})", label = NULL) { @@ -95,7 +94,7 @@ tbl_continuous <- function(data, digits = digits ) - type <- assign_summary_type(data, variables = include, value = NULL) + type <- rlang::rep_named(include, list("categorical")) # save processed function inputs --------------------------------------------- tbl_continuous_inputs <- as.list(environment()) diff --git a/man/tbl_strata.Rd b/man/tbl_strata.Rd index 9a7ea24e70..807b38950d 100644 --- a/man/tbl_strata.Rd +++ b/man/tbl_strata.Rd @@ -67,7 +67,7 @@ curly brackets will be evaluated according to \code{glue::glue()} syntax. The evaluated value of \code{.header} is also available within \code{tbl_strata2(.tbl_fun)}} -\item{.stack_group_header}{DEPRECATED.} +\item{.stack_group_header}{\ifelse{html}{\href{https://lifecycle.r-lib.org/articles/stages.html#deprecated}{\figure{lifecycle-deprecated.svg}{options: alt='[Deprecated]'}}}{\strong{[Deprecated]}}} \item{.quiet}{\ifelse{html}{\href{https://lifecycle.r-lib.org/articles/stages.html#deprecated}{\figure{lifecycle-deprecated.svg}{options: alt='[Deprecated]'}}}{\strong{[Deprecated]}}} } diff --git a/tests/testthat/test-add_stat.tbl_continuous.R b/tests/testthat/test-add_stat.tbl_continuous.R index 9f93e11501..98ae70d5ae 100644 --- a/tests/testthat/test-add_stat.tbl_continuous.R +++ b/tests/testthat/test-add_stat.tbl_continuous.R @@ -8,21 +8,49 @@ test_that("add_stat for tbl_continuous() works", { ) add_stat_test1 <- function(data, variable, by, ...) { - tibble::tibble(test_col = "Ugh") + tibble::tibble(addtl = "Data from elsewhere") } + expect_equal( + tt %>% + add_stat(fns = everything() ~ add_stat_test1) %>% + as_tibble() %>% + dplyr::pull(addtl), + c("Data from elsewhere", NA, NA, NA) + ) +}) + + +test_that("add_stat for tbl_continuous() works", { + tt <- + trial %>% + tbl_continuous( + age, + include = grade, + by = trt + ) + + p_vals <- lapply( + dplyr::group_split(trial, grade), + function(x) t.test(x$age ~ x$trt)$p.value + ) %>% + unlist %>% + round(3) %>% + as.character + add_stat_test2 <- function(data, variable, by, ...) { - tibble::tibble(test_col = rep_len("Ugh", 3)) + lapply( + dplyr::group_split(trial, grade), + function(x) t.test(x$age ~ x$trt)$p.value + ) %>% + unlist } - expect_error( - tt %>% - add_stat(fns = everything() ~ add_stat_test1), - NA - ) - expect_error( + expect_equal( tt %>% - add_stat(fns = everything() ~ add_stat_test2, location = everything() ~ "level"), - NA + add_stat(fns = everything() ~ add_stat_test2, location = everything() ~ "level") %>% + as_tibble() %>% + dplyr::pull(add_stat_1), + c(NA, p_vals) ) }) From 02400095f6da4ce6abb882956bac871a520ddcd3 Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Tue, 25 Jun 2024 16:05:54 +0000 Subject: [PATCH 05/11] as a fun exercise --- tests/testthat/test-add_stat.tbl_continuous.R | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tests/testthat/test-add_stat.tbl_continuous.R b/tests/testthat/test-add_stat.tbl_continuous.R index 98ae70d5ae..cdfba1a42d 100644 --- a/tests/testthat/test-add_stat.tbl_continuous.R +++ b/tests/testthat/test-add_stat.tbl_continuous.R @@ -38,12 +38,13 @@ test_that("add_stat for tbl_continuous() works", { round(3) %>% as.character - add_stat_test2 <- function(data, variable, by, ...) { - lapply( - dplyr::group_split(trial, grade), - function(x) t.test(x$age ~ x$trt)$p.value - ) %>% - unlist + add_stat_test2 <- function(data, variable, by, tbl, ...) { + col_name <- tbl$inputs$include + col_sym <- sym(col_name) + + strat <- dplyr::group_split(data, !!col_sym) + + lapply(strat, function(x) t.test(x$age ~ x$trt)$p.value) %>% unlist } expect_equal( From 9bdb68dd5905d88b6cc5e0e758cd9177369afe3d Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Tue, 25 Jun 2024 16:08:20 +0000 Subject: [PATCH 06/11] add comments --- tests/testthat/test-add_stat.tbl_continuous.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testthat/test-add_stat.tbl_continuous.R b/tests/testthat/test-add_stat.tbl_continuous.R index cdfba1a42d..8c2fcc6c0a 100644 --- a/tests/testthat/test-add_stat.tbl_continuous.R +++ b/tests/testthat/test-add_stat.tbl_continuous.R @@ -21,7 +21,7 @@ test_that("add_stat for tbl_continuous() works", { }) -test_that("add_stat for tbl_continuous() works", { +test_that("add_stat for tbl_continuous() works using location", { tt <- trial %>% tbl_continuous( From 7b37b2766b5b12fdf18ebd0797d47f8877d11ed5 Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Tue, 25 Jun 2024 17:05:30 +0000 Subject: [PATCH 07/11] nvm found the issue --- R/tbl_continuous.R | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/R/tbl_continuous.R b/R/tbl_continuous.R index e1029bc8c2..f70e9a1010 100644 --- a/R/tbl_continuous.R +++ b/R/tbl_continuous.R @@ -94,8 +94,6 @@ tbl_continuous <- function(data, digits = digits ) - type <- rlang::rep_named(include, list("categorical")) - # save processed function inputs --------------------------------------------- tbl_continuous_inputs <- as.list(environment()) call <- match.call() @@ -190,6 +188,7 @@ tbl_continuous <- function(data, # add other information to the returned object x$cards <- list(tbl_continuous = cards) x$inputs <- tbl_continuous_inputs + x$inputs$type <- rlang::rep_named(include, list("categorical")) x$call_list <- list(tbl_continuous = call) x |> From 4ca40c7bb413e42f40d37519b7eb0c7d4f5d6848 Mon Sep 17 00:00:00 2001 From: Daniel Sjoberg Date: Tue, 25 Jun 2024 11:44:58 -0700 Subject: [PATCH 08/11] DS review updates --- R/add_stat.R | 6 +++++- R/tbl_continuous.R | 1 - tests/testthat/test-add_stat.tbl_continuous.R | 8 ++++---- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/R/add_stat.R b/R/add_stat.R index bd2141eb58..efb95f25cd 100644 --- a/R/add_stat.R +++ b/R/add_stat.R @@ -114,6 +114,11 @@ add_stat <- function(x, fns, location = everything() ~ "label") { check_not_missing(fns) check_class(x, c("tbl_summary", "tbl_svysummary", "tbl_continuous")) + # adding type if `tbl_continuous`...this is used later on + if (inherits(x, "tbl_continuous")) { + x$inputs$type <- rep_named(x$inputs$include, list("categorical")) + } + # convert to named lists ----------------------------------------------------- cards::process_formula_selectors( select_prep(x$table_body), @@ -233,7 +238,6 @@ eval_fn_safe <- function(variable, tbl, fn) { data = tbl$inputs$data, variable = variable, by = tbl$inputs$by, - include = tbl$inputs$include, tbl = tbl ) ) diff --git a/R/tbl_continuous.R b/R/tbl_continuous.R index f70e9a1010..f850ff8896 100644 --- a/R/tbl_continuous.R +++ b/R/tbl_continuous.R @@ -188,7 +188,6 @@ tbl_continuous <- function(data, # add other information to the returned object x$cards <- list(tbl_continuous = cards) x$inputs <- tbl_continuous_inputs - x$inputs$type <- rlang::rep_named(include, list("categorical")) x$call_list <- list(tbl_continuous = call) x |> diff --git a/tests/testthat/test-add_stat.tbl_continuous.R b/tests/testthat/test-add_stat.tbl_continuous.R index 8c2fcc6c0a..d389a227e6 100644 --- a/tests/testthat/test-add_stat.tbl_continuous.R +++ b/tests/testthat/test-add_stat.tbl_continuous.R @@ -1,6 +1,6 @@ -test_that("add_stat for tbl_continuous() works", { +test_that("add_stat() for 'tbl_continuous'", { tt <- - trial %>% + trial |> tbl_continuous( age, include = grade, @@ -12,7 +12,7 @@ test_that("add_stat for tbl_continuous() works", { } expect_equal( - tt %>% + tt |> add_stat(fns = everything() ~ add_stat_test1) %>% as_tibble() %>% dplyr::pull(addtl), @@ -21,7 +21,7 @@ test_that("add_stat for tbl_continuous() works", { }) -test_that("add_stat for tbl_continuous() works using location", { +test_that("add_stat(location) for 'tbl_continuous'", { tt <- trial %>% tbl_continuous( From 51c2151e383b0a64041d4855e1aa187b088d0cdb Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Tue, 25 Jun 2024 20:37:13 +0000 Subject: [PATCH 09/11] move the tests and take in feedback --- tests/testthat/test-add_stat.R | 57 ++++++++++++++++++- tests/testthat/test-add_stat.tbl_continuous.R | 57 ------------------- 2 files changed, 56 insertions(+), 58 deletions(-) delete mode 100644 tests/testthat/test-add_stat.tbl_continuous.R diff --git a/tests/testthat/test-add_stat.R b/tests/testthat/test-add_stat.R index 4786dd7df2..338f3e5fdd 100644 --- a/tests/testthat/test-add_stat.R +++ b/tests/testthat/test-add_stat.R @@ -170,4 +170,59 @@ test_that("add_stat(x) messaging", { # TODO: Add `tbl_svysummary()` tests -# TODO: Add `tbl_continuous()` tests +# `tbl_continuous()` tests +test_that("add_stat() for 'tbl_continuous'", { + tt <- + trial |> + tbl_continuous( + age, + include = grade, + by = trt + ) + + add_stat_test1 <- function(data, variable, by, ...) { + tibble::tibble(addtl = "Data from elsewhere") + } + + expect_equal( + tt |> + add_stat(fns = everything() ~ add_stat_test1) |> + as_tibble() |> + dplyr::pull(addtl), + c("Data from elsewhere", NA, NA, NA) + ) +}) + + +test_that("add_stat(location) for 'tbl_continuous'", { + tt <- + trial |> + tbl_continuous( + age, + include = grade, + by = trt + ) + + p_vals <- lapply( + dplyr::group_split(trial, grade), + function(x) t.test(x$age ~ x$trt)$p.value + ) %>% + unlist() |> + round(3) %>% + as.character() + + add_stat_test2 <- function(data, variable, by, tbl, ...) { + data |> + dplyr::group_split(!!sym(tbl$inputs$include)) |> + map_dbl(~ t.test(.x[[tbl$inputs$variable]] ~ .x[[by]])$p.value) + } + + expect_equal( + tt |> + add_stat(fns = everything() ~ add_stat_test2, location = everything() ~ "level") |> + as_tibble() |> + dplyr::pull(add_stat_1), + c(NA, p_vals) + ) +}) + diff --git a/tests/testthat/test-add_stat.tbl_continuous.R b/tests/testthat/test-add_stat.tbl_continuous.R deleted file mode 100644 index d389a227e6..0000000000 --- a/tests/testthat/test-add_stat.tbl_continuous.R +++ /dev/null @@ -1,57 +0,0 @@ -test_that("add_stat() for 'tbl_continuous'", { - tt <- - trial |> - tbl_continuous( - age, - include = grade, - by = trt - ) - - add_stat_test1 <- function(data, variable, by, ...) { - tibble::tibble(addtl = "Data from elsewhere") - } - - expect_equal( - tt |> - add_stat(fns = everything() ~ add_stat_test1) %>% - as_tibble() %>% - dplyr::pull(addtl), - c("Data from elsewhere", NA, NA, NA) - ) -}) - - -test_that("add_stat(location) for 'tbl_continuous'", { - tt <- - trial %>% - tbl_continuous( - age, - include = grade, - by = trt - ) - - p_vals <- lapply( - dplyr::group_split(trial, grade), - function(x) t.test(x$age ~ x$trt)$p.value - ) %>% - unlist %>% - round(3) %>% - as.character - - add_stat_test2 <- function(data, variable, by, tbl, ...) { - col_name <- tbl$inputs$include - col_sym <- sym(col_name) - - strat <- dplyr::group_split(data, !!col_sym) - - lapply(strat, function(x) t.test(x$age ~ x$trt)$p.value) %>% unlist - } - - expect_equal( - tt %>% - add_stat(fns = everything() ~ add_stat_test2, location = everything() ~ "level") %>% - as_tibble() %>% - dplyr::pull(add_stat_1), - c(NA, p_vals) - ) -}) From 3fc29f2d412a90b1b071937e31d2159dbb48b3bf Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Tue, 25 Jun 2024 20:44:20 +0000 Subject: [PATCH 10/11] remove unncessary comments and return tbl_continuous back to original shape --- R/add_stat.R | 2 -- R/tbl_continuous.R | 1 + 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/R/add_stat.R b/R/add_stat.R index efb95f25cd..5d69d05a31 100644 --- a/R/add_stat.R +++ b/R/add_stat.R @@ -153,8 +153,6 @@ add_stat <- function(x, fns, location = everything() ~ "label") { dplyr::tibble(variable = names(fns)) |> dplyr::mutate( summary_type = map_chr(.data$variable, ~ x$inputs$type[[.x]]), - # tbl_continuous objects generally didn't have a "$inputs$type" object nested inside - # added it back to tbl_continuous row_type = map_chr(.data$variable, ~ location[[.x]]), label = map2( .data$variable, .data$row_type, diff --git a/R/tbl_continuous.R b/R/tbl_continuous.R index f850ff8896..758f454ebb 100644 --- a/R/tbl_continuous.R +++ b/R/tbl_continuous.R @@ -94,6 +94,7 @@ tbl_continuous <- function(data, digits = digits ) + # save processed function inputs --------------------------------------------- tbl_continuous_inputs <- as.list(environment()) call <- match.call() From 61d78119fb09badb33f0b67a0fed975c9d1d4b15 Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Tue, 25 Jun 2024 20:51:09 +0000 Subject: [PATCH 11/11] use native pipe only --- tests/testthat/test-add_stat.R | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/testthat/test-add_stat.R b/tests/testthat/test-add_stat.R index 338f3e5fdd..644ac3f70b 100644 --- a/tests/testthat/test-add_stat.R +++ b/tests/testthat/test-add_stat.R @@ -203,12 +203,10 @@ test_that("add_stat(location) for 'tbl_continuous'", { by = trt ) - p_vals <- lapply( - dplyr::group_split(trial, grade), - function(x) t.test(x$age ~ x$trt)$p.value - ) %>% - unlist() |> - round(3) %>% + p_vals <- trial |> + dplyr::group_split(grade) |> + map_dbl(~ t.test(.x[["age"]] ~ .x[["trt"]])$p.value)|> + round(3) |> as.character() add_stat_test2 <- function(data, variable, by, tbl, ...) {