From 03d8639fb12ca8334b16879424d712c6e90e9b49 Mon Sep 17 00:00:00 2001 From: Emily de la Rua <59304861+edelarua@users.noreply.github.com> Date: Thu, 5 Sep 2024 10:19:09 -0400 Subject: [PATCH] Add warning for missing p-value in `tabulate_rsp_subgroups()` output (#1296) Fixes #1289 --- NEWS.md | 2 +- R/argument_convention.R | 2 +- R/response_subgroups.R | 8 ++++++++ man/argument_convention.Rd | 2 +- man/d_rsp_subgroups_colvars.Rd | 2 +- man/extract_rsp_subgroups.Rd | 2 +- man/h_response_subgroups.Rd | 2 +- tests/testthat/test-response_subgroups.R | 21 +++++++++++++++++++++ 8 files changed, 35 insertions(+), 6 deletions(-) diff --git a/NEWS.md b/NEWS.md index e63b36871d..01cd53d85d 100644 --- a/NEWS.md +++ b/NEWS.md @@ -4,6 +4,7 @@ * Reworking of `summarize_glm_count()` documentation and all its associated functions to better describe the results and the functions' purpose. * Added the `.formats` argument to `tabulate_rsp_subgroups` and `tabulate_survival_subgroups` to allow users to specify formats. * Added the `riskdiff` argument to `tabulate_rsp_subgroups` and `tabulate_survival_subgroups` to allow users to add a risk difference table column, and function `control_riskdiff` to specify settings for the risk difference column. +* Added warning to `tabulate_rsp_subgroups` when `pval` statistic is selected but `df` has not been correctly generated to add p-values to the output table. ### Bug Fixes * Fixed a bug in `a_surv_time` that threw an error when split only has `"is_event"`. @@ -15,7 +16,6 @@ * Fixed bug in `decorate_grob` that did not handle well empty strings or `NULL` values for title and footers. * Fixed bug in `g_km` that caused an error when multiple records in the data had estimates at max time. - ### Miscellaneous * Began deprecation of the confusing functions `summary_formats` and `summary_labels`. diff --git a/R/argument_convention.R b/R/argument_convention.R index bc6a7220d8..34c92b7786 100644 --- a/R/argument_convention.R +++ b/R/argument_convention.R @@ -43,7 +43,7 @@ #' for more information. #' @param lyt (`PreDataTableLayouts`)\cr layout that analyses will be added to. #' @param method (`string` or `NULL`)\cr specifies the test used to calculate the p-value for the difference between -#' two proportions. For options, see [s_test_proportion_diff()]. Default is `NULL` so no test is performed. +#' two proportions. For options, see [test_proportion_diff()]. Default is `NULL` so no test is performed. #' @param na.rm (`flag`)\cr whether `NA` values should be removed from `x` prior to analysis. #' @param na_str (`string`)\cr string used to replace all `NA` or empty values in the output. #' @param nested (`flag`)\cr whether this layout instruction should be applied within the existing layout structure _if diff --git a/R/response_subgroups.R b/R/response_subgroups.R index 3f4cdb474d..c24606c58e 100644 --- a/R/response_subgroups.R +++ b/R/response_subgroups.R @@ -220,6 +220,14 @@ tabulate_rsp_subgroups <- function(lyt, )) { checkmate::assert_list(riskdiff, null.ok = TRUE) checkmate::assert_true(all(c("n_tot", "or", "ci") %in% vars)) + if ("pval" %in% vars && !"pval" %in% names(df$or)) { + warning( + 'The "pval" statistic has been selected but is not present in "df" so it will not be included in the output ', + 'table. To include the "pval" statistic, please specify a p-value test when generating "df" via ', + 'the "method" argument to `extract_rsp_subgroups()`. If method = "cmh", strata must also be specified via the ', + '"variables" argument to `extract_rsp_subgroups()`.' + ) + } # Create "ci" column from "lcl" and "ucl" df$or$ci <- combine_vectors(df$or$lcl, df$or$ucl) diff --git a/man/argument_convention.Rd b/man/argument_convention.Rd index 8f50bec33e..7b13f98616 100644 --- a/man/argument_convention.Rd +++ b/man/argument_convention.Rd @@ -70,7 +70,7 @@ for more information.} \item{lyt}{(\code{PreDataTableLayouts})\cr layout that analyses will be added to.} \item{method}{(\code{string} or \code{NULL})\cr specifies the test used to calculate the p-value for the difference between -two proportions. For options, see \code{\link[=s_test_proportion_diff]{s_test_proportion_diff()}}. Default is \code{NULL} so no test is performed.} +two proportions. For options, see \code{\link[=test_proportion_diff]{test_proportion_diff()}}. Default is \code{NULL} so no test is performed.} \item{na.rm}{(\code{flag})\cr whether \code{NA} values should be removed from \code{x} prior to analysis.} diff --git a/man/d_rsp_subgroups_colvars.Rd b/man/d_rsp_subgroups_colvars.Rd index ca9f5af0b4..f16cb0ccc7 100644 --- a/man/d_rsp_subgroups_colvars.Rd +++ b/man/d_rsp_subgroups_colvars.Rd @@ -12,7 +12,7 @@ d_rsp_subgroups_colvars(vars, conf_level = NULL, method = NULL) \item{conf_level}{(\code{proportion})\cr confidence level of the interval.} \item{method}{(\code{string} or \code{NULL})\cr specifies the test used to calculate the p-value for the difference between -two proportions. For options, see \code{\link[=s_test_proportion_diff]{s_test_proportion_diff()}}. Default is \code{NULL} so no test is performed.} +two proportions. For options, see \code{\link[=test_proportion_diff]{test_proportion_diff()}}. Default is \code{NULL} so no test is performed.} } \value{ A \code{list} of variables to tabulate and their labels. diff --git a/man/extract_rsp_subgroups.Rd b/man/extract_rsp_subgroups.Rd index 37e5239dbc..538de425d6 100644 --- a/man/extract_rsp_subgroups.Rd +++ b/man/extract_rsp_subgroups.Rd @@ -25,7 +25,7 @@ levels that belong to it in the character vectors that are elements of the list. \item{conf_level}{(\code{proportion})\cr confidence level of the interval.} \item{method}{(\code{string} or \code{NULL})\cr specifies the test used to calculate the p-value for the difference between -two proportions. For options, see \code{\link[=s_test_proportion_diff]{s_test_proportion_diff()}}. Default is \code{NULL} so no test is performed.} +two proportions. For options, see \code{\link[=test_proportion_diff]{test_proportion_diff()}}. Default is \code{NULL} so no test is performed.} \item{label_all}{(\code{string})\cr label for the total population analysis.} } diff --git a/man/h_response_subgroups.Rd b/man/h_response_subgroups.Rd index 517a602c94..2e07f2da13 100644 --- a/man/h_response_subgroups.Rd +++ b/man/h_response_subgroups.Rd @@ -48,7 +48,7 @@ levels that belong to it in the character vectors that are elements of the list. \item{conf_level}{(\code{proportion})\cr confidence level of the interval.} \item{method}{(\code{string} or \code{NULL})\cr specifies the test used to calculate the p-value for the difference between -two proportions. For options, see \code{\link[=s_test_proportion_diff]{s_test_proportion_diff()}}. Default is \code{NULL} so no test is performed.} +two proportions. For options, see \code{\link[=test_proportion_diff]{test_proportion_diff()}}. Default is \code{NULL} so no test is performed.} } \value{ \itemize{ diff --git a/tests/testthat/test-response_subgroups.R b/tests/testthat/test-response_subgroups.R index 3d8a9ed812..7422d7c64e 100644 --- a/tests/testthat/test-response_subgroups.R +++ b/tests/testthat/test-response_subgroups.R @@ -307,3 +307,24 @@ testthat::test_that("tabulate_rsp_subgroups riskdiff argument works as expected" res <- testthat::expect_silent(result) testthat::expect_snapshot(res) }) + +testthat::test_that("tabulate_rsp_subgroups pval statistic warning works as expected", { + adrs <- adrs_200 + + df <- extract_rsp_subgroups( + variables = list(rsp = "rsp", arm = "ARM", subgroups = c("SEX", "STRATA2")), + data = adrs, + method = NULL, + conf_level = 0.95 + ) + + # warning when no pval in df + expect_warning( + basic_table() %>% + tabulate_rsp_subgroups( + df = df, + vars = c("n", "prop", "n_tot", "or", "ci", "pval") + ), + "please specify a p-value test" + ) +})