From 599a66048260ea39addcd0c5be88b47d12935671 Mon Sep 17 00:00:00 2001 From: Emily de la Rua Date: Fri, 4 Oct 2024 17:01:30 -0400 Subject: [PATCH 01/14] Add method parameter to s_odds_ratio and estimate_odds_Ratio --- R/odds_ratio.R | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/R/odds_ratio.R b/R/odds_ratio.R index eccdccc1b1..4911a1de61 100644 --- a/R/odds_ratio.R +++ b/R/odds_ratio.R @@ -13,6 +13,8 @@ #' @inheritParams argument_convention #' @param .stats (`character`)\cr statistics to select for the table. Run `get_stats("estimate_odds_ratio")` #' to see available statistics for this function. +#' @param method (`string`)\cr whether to use the correct (`"exact"`) calculation in the conditional likelihood or one +#' of the approximations. See [survival::clogit()] for details. #' #' @note #' * This function uses logistic regression for unstratified analyses, and conditional logistic regression for @@ -64,7 +66,8 @@ s_odds_ratio <- function(df, .df_row, variables = list(arm = NULL, strata = NULL), conf_level = 0.95, - groups_list = NULL) { + groups_list = NULL, + method = "exact") { y <- list(or_ci = "", n_tot = "") if (!.in_ref_col) { @@ -83,6 +86,7 @@ s_odds_ratio <- function(df, y <- or_glm(data, conf_level = conf_level) } else { assert_df_with_variables(.df_row, c(list(rsp = .var), variables)) + checkmate::assert_subset(method, c("exact", "approximate", "efron", "breslow"), empty.ok = FALSE) # The group variable prepared for clogit must be synchronised with combination groups definition. if (is.null(groups_list)) { @@ -118,7 +122,7 @@ s_odds_ratio <- function(df, grp = grp, strata = interaction(.df_row[variables$strata]) ) - y_all <- or_clogit(data, conf_level = conf_level) + y_all <- or_clogit(data, conf_level = conf_level, method = method) checkmate::assert_string(trt_grp) checkmate::assert_subset(trt_grp, names(y_all$or_ci)) y$or_ci <- y_all$or_ci[[trt_grp]] @@ -193,7 +197,7 @@ estimate_odds_ratio <- function(lyt, groups_list = NULL, na_str = default_na_str(), nested = TRUE, - ..., + method = "exact", show_labels = "hidden", table_names = vars, var_labels = vars, @@ -201,7 +205,7 @@ estimate_odds_ratio <- function(lyt, .formats = NULL, .labels = NULL, .indent_mods = NULL) { - extra_args <- list(variables = variables, conf_level = conf_level, groups_list = groups_list, ...) + extra_args <- list(variables = variables, conf_level = conf_level, groups_list = groups_list, method = method) afun <- make_afun( a_odds_ratio, @@ -230,6 +234,7 @@ estimate_odds_ratio <- function(lyt, #' #' Functions to calculate odds ratios in [estimate_odds_ratio()]. #' +#' @inheritParams odds_ratio #' @inheritParams argument_convention #' @param data (`data.frame`)\cr data frame containing at least the variables `rsp` and `grp`, and optionally #' `strata` for [or_clogit()]. @@ -300,19 +305,20 @@ or_glm <- function(data, conf_level) { #' or_clogit(data, conf_level = 0.95) #' #' @export -or_clogit <- function(data, conf_level) { +or_clogit <- function(data, conf_level, method = "exact") { checkmate::assert_logical(data$rsp) assert_proportion_value(conf_level) assert_df_with_variables(data, list(rsp = "rsp", grp = "grp", strata = "strata")) checkmate::assert_multi_class(data$grp, classes = c("factor", "character")) checkmate::assert_multi_class(data$strata, classes = c("factor", "character")) + checkmate::assert_subset(method, c("exact", "approximate", "efron", "breslow"), empty.ok = FALSE) data$grp <- as_factor_keep_attributes(data$grp) data$strata <- as_factor_keep_attributes(data$strata) # Deviation from convention: `survival::strata` must be simply `strata`. formula <- stats::as.formula("rsp ~ grp + strata(strata)") - model_fit <- clogit_with_tryCatch(formula = formula, data = data) + model_fit <- clogit_with_tryCatch(formula = formula, data = data, method = method) # Create a list with one set of OR estimates and CI per coefficient, i.e. # comparison of one group vs. the reference group. From 4055998dcf702083e4bb2be170a329d126ff485a Mon Sep 17 00:00:00 2001 From: Emily de la Rua Date: Fri, 4 Oct 2024 17:02:19 -0400 Subject: [PATCH 02/14] Add tests --- tests/testthat/_snaps/odds_ratio.md | 9 ++++++ tests/testthat/test-odds_ratio.R | 46 +++++++++++++++++++++++++++-- 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/tests/testthat/_snaps/odds_ratio.md b/tests/testthat/_snaps/odds_ratio.md index 3614b53e64..3bbb44dd2d 100644 --- a/tests/testthat/_snaps/odds_ratio.md +++ b/tests/testthat/_snaps/odds_ratio.md @@ -97,3 +97,12 @@ ——————————————————————————————————————————————————————————— Odds Ratio (95% CI) 1.24 (0.54 - 2.89) +# estimate_odds_ratio method argument works + + Code + res + Output + A B + ———————————————————————————————————————————— + Odds Ratio (95% CI) 0.96 (0.85 - 1.08) + diff --git a/tests/testthat/test-odds_ratio.R b/tests/testthat/test-odds_ratio.R index 1cbd34c79d..6190a9ed21 100644 --- a/tests/testthat/test-odds_ratio.R +++ b/tests/testthat/test-odds_ratio.R @@ -172,10 +172,52 @@ testthat::test_that("estimate_odds_ratio works with strata and combined groups", # https://github.com/therneau/survival/issues/240 withr::with_options( - opts_partial_match_old, - result <- build_table(lyt = lyt, df = anl) + res <- testthat::expect_silent(result) + testthat::expect_snapshot(res) +}) + +testthat::test_that("s_odds_ratio method argument works", { + nex <- 2000 # Number of example rows + dta <- data.frame( + "rsp" = sample(c(TRUE, FALSE), nex, TRUE), + "grp" = sample(c("A", "B"), nex, TRUE), + "f1" = sample(c("a1", "a2"), nex, TRUE), + "f2" = sample(c("x", "y", "z"), nex, TRUE), + strata = factor(sample(c("C", "D"), nex, TRUE)), + stringsAsFactors = TRUE + ) + + res <- s_odds_ratio( + df = subset(dta, grp == "A"), + .var = "rsp", + .ref_group = subset(dta, grp == "B"), + .in_ref_col = FALSE, + .df_row = dta, + variables = list(arm = "grp", strata = "strata"), + method = "approximate" ) + testthat::expect_false(all(is.na(res$or_ci))) +}) + +testthat::test_that("estimate_odds_ratio method argument works", { + nex <- 2000 # Number of example rows + set.seed(12) + dta <- data.frame( + "rsp" = sample(c(TRUE, FALSE), nex, TRUE), + "grp" = sample(c("A", "B"), nex, TRUE), + "f1" = sample(c("a1", "a2"), nex, TRUE), + "f2" = sample(c("x", "y", "z"), nex, TRUE), + strata = factor(sample(c("C", "D"), nex, TRUE)), + stringsAsFactors = TRUE + ) + + lyt <- basic_table() %>% + split_cols_by(var = "grp", ref_group = "B") %>% + estimate_odds_ratio(vars = "rsp", variables = list(arm = "grp", strata = "strata"), method = "approximate") + + result <- build_table(lyt, df = dta) + res <- testthat::expect_silent(result) testthat::expect_snapshot(res) }) From 96e6f8fa1ff94f5babd6eba5f371e1d31b856567 Mon Sep 17 00:00:00 2001 From: Emily de la Rua Date: Fri, 4 Oct 2024 17:02:53 -0400 Subject: [PATCH 03/14] Increase survival version, remove param workarounds from tests --- DESCRIPTION | 2 +- tests/testthat/test-odds_ratio.R | 62 ++++++++++++-------------------- 2 files changed, 24 insertions(+), 40 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index ff84f8d014..23d0254bd6 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -48,7 +48,7 @@ Imports: rlang (>= 1.1.0), scales (>= 1.2.0), stats, - survival (>= 3.2-13), + survival (>= 3.7-0), tibble (>= 2.0.0), tidyr (>= 0.8.3), utils diff --git a/tests/testthat/test-odds_ratio.R b/tests/testthat/test-odds_ratio.R index 6190a9ed21..e66535f97f 100644 --- a/tests/testthat/test-odds_ratio.R +++ b/tests/testthat/test-odds_ratio.R @@ -30,11 +30,7 @@ testthat::test_that("or_clogit estimates right OR and CI", { stringsAsFactors = TRUE ) - # https://github.com/therneau/survival/issues/240 - withr::with_options( - opts_partial_match_old, - result <- or_clogit(data, conf_level = 0.95) - ) + result <- or_clogit(data, conf_level = 0.95) # from SAS res <- testthat::expect_silent(result) @@ -66,17 +62,13 @@ testthat::test_that("s_odds_ratio estimates right OR and CI (stratified analysis strata = factor(sample(c("C", "D"), 100, TRUE)) ) - # https://github.com/therneau/survival/issues/240 - withr::with_options( - opts_partial_match_old, - result <- s_odds_ratio( - df = subset(dta, grp == "A"), - .var = "rsp", - .ref_group = subset(dta, grp == "B"), - .in_ref_col = FALSE, - .df_row = dta, - variables = list(arm = "grp", strata = "strata") - ) + result <- s_odds_ratio( + df = subset(dta, grp == "A"), + .var = "rsp", + .ref_group = subset(dta, grp == "B"), + .in_ref_col = FALSE, + .df_row = dta, + variables = list(arm = "grp", strata = "strata") ) res <- testthat::expect_silent(result) @@ -94,19 +86,15 @@ testthat::test_that("s_odds_ratio returns error for incorrect groups", { "Arms A+B" = c("A", "B") ) - # https://github.com/therneau/survival/issues/240 - withr::with_options( - opts_partial_match_old, - result <- testthat::expect_error(s_odds_ratio( - df = subset(data, grp == "A"), - .var = "rsp", - .ref_group = subset(data, grp == "B"), - .in_ref_col = FALSE, - .df_row = data, - variables = list(arm = "grp", strata = "strata"), - groups_list = groups - )) - ) + testthat::expect_error(result <- s_odds_ratio( + df = subset(data, grp == "A"), + .var = "rsp", + .ref_group = subset(data, grp == "B"), + .in_ref_col = FALSE, + .df_row = data, + variables = list(arm = "grp", strata = "strata"), + groups_list = groups + )) }) testthat::test_that("estimate_odds_ratio estimates right OR and CI (unstratified analysis)", { @@ -132,14 +120,10 @@ testthat::test_that("estimate_odds_ratio estimates right OR and CI (stratified a strata = factor(sample(c("C", "D"), 100, TRUE)) ) - # https://github.com/therneau/survival/issues/240 - withr::with_options( - opts_partial_match_old, - result <- basic_table() %>% - split_cols_by(var = "grp", ref_group = "A", split_fun = ref_group_position("first")) %>% - estimate_odds_ratio(vars = "rsp", variables = list(arm = "grp", strata = "strata")) %>% - build_table(df = data) - ) + result <- basic_table() %>% + split_cols_by(var = "grp", ref_group = "A", split_fun = ref_group_position("first")) %>% + estimate_odds_ratio(vars = "rsp", variables = list(arm = "grp", strata = "strata")) %>% + build_table(df = data) res <- testthat::expect_silent(result) testthat::expect_snapshot(res) @@ -170,8 +154,8 @@ testthat::test_that("estimate_odds_ratio works with strata and combined groups", groups_list = groups ) - # https://github.com/therneau/survival/issues/240 - withr::with_options( + result <- build_table(lyt = lyt, df = anl) + res <- testthat::expect_silent(result) testthat::expect_snapshot(res) }) From 9f3a3318d3a3549afd9428e82ef9cc2f699316ae Mon Sep 17 00:00:00 2001 From: Emily de la Rua Date: Fri, 4 Oct 2024 17:03:02 -0400 Subject: [PATCH 04/14] Update docs --- man/h_odds_ratio.Rd | 5 ++++- man/odds_ratio.Rd | 13 +++++++++---- man/try_car_anova.Rd | 4 ++-- 3 files changed, 15 insertions(+), 7 deletions(-) diff --git a/man/h_odds_ratio.Rd b/man/h_odds_ratio.Rd index 238c2d7c86..ca24b33a8a 100644 --- a/man/h_odds_ratio.Rd +++ b/man/h_odds_ratio.Rd @@ -8,13 +8,16 @@ \usage{ or_glm(data, conf_level) -or_clogit(data, conf_level) +or_clogit(data, conf_level, method = "exact") } \arguments{ \item{data}{(\code{data.frame})\cr data frame containing at least the variables \code{rsp} and \code{grp}, and optionally \code{strata} for \code{\link[=or_clogit]{or_clogit()}}.} \item{conf_level}{(\code{proportion})\cr confidence level of the interval.} + +\item{method}{(\code{string})\cr whether to use the correct (\code{"exact"}) calculation in the conditional likelihood or one +of the approximations. See \code{\link[survival:clogit]{survival::clogit()}} for details.} } \value{ A named \code{list} of elements \code{or_ci} and \code{n_tot}. diff --git a/man/odds_ratio.Rd b/man/odds_ratio.Rd index ee88612e3b..7d25f40ff5 100644 --- a/man/odds_ratio.Rd +++ b/man/odds_ratio.Rd @@ -15,7 +15,7 @@ estimate_odds_ratio( groups_list = NULL, na_str = default_na_str(), nested = TRUE, - ..., + method = "exact", show_labels = "hidden", table_names = vars, var_labels = vars, @@ -33,7 +33,8 @@ s_odds_ratio( .df_row, variables = list(arm = NULL, strata = NULL), conf_level = 0.95, - groups_list = NULL + groups_list = NULL, + method = "exact" ) a_odds_ratio( @@ -44,7 +45,8 @@ a_odds_ratio( .df_row, variables = list(arm = NULL, strata = NULL), conf_level = 0.95, - groups_list = NULL + groups_list = NULL, + method = "exact" ) } \arguments{ @@ -65,7 +67,8 @@ levels that belong to it in the character vectors that are elements of the list. possible (\code{TRUE}, the default) or as a new top-level element (\code{FALSE}). Ignored if it would nest a split. underneath analyses, which is not allowed.} -\item{...}{arguments passed to \code{s_odds_ratio()}.} +\item{method}{(\code{string})\cr whether to use the correct (\code{"exact"}) calculation in the conditional likelihood or one +of the approximations. See \code{\link[survival:clogit]{survival::clogit()}} for details.} \item{show_labels}{(\code{string})\cr label visibility: one of "default", "visible" and "hidden".} @@ -95,6 +98,8 @@ by a statistics function.} \item{.in_ref_col}{(\code{flag})\cr \code{TRUE} when working with the reference level, \code{FALSE} otherwise.} \item{.df_row}{(\code{data.frame})\cr data frame across all of the columns for the given row split.} + +\item{...}{arguments passed to \code{s_odds_ratio()}.} } \value{ \itemize{ diff --git a/man/try_car_anova.Rd b/man/try_car_anova.Rd index b3ba87bf83..a81cde8287 100644 --- a/man/try_car_anova.Rd +++ b/man/try_car_anova.Rd @@ -9,12 +9,12 @@ try_car_anova(mod, test.statistic) \arguments{ \item{mod}{\code{lm}, \code{aov}, \code{glm}, \code{multinom}, \code{polr} \code{mlm}, \code{coxph}, \code{coxme}, \code{lme}, \code{mer}, \code{merMod}, \code{svyglm}, \code{svycoxph}, - \code{rlm}, \code{clm}, \code{clmm}, or other suitable model object.} + \code{rlm}, or other suitable model object.} \item{test.statistic}{for a generalized linear model, whether to calculate \code{"LR"} (likelihood-ratio), \code{"Wald"}, or \code{"F"} tests; for a Cox or Cox mixed-effects model, whether to calculate \code{"LR"} (partial-likelihood ratio) or - \code{"Wald"} tests (with \code{"LR"} tests unavailable for Cox models using the \code{tt} argument); in the default case or for linear mixed models fit by + \code{"Wald"} tests; in the default case or for linear mixed models fit by \code{lmer}, whether to calculate Wald \code{"Chisq"} or Kenward-Roger \code{"F"} tests with Satterthwaite degrees of freedom (\emph{warning:} the KR F-tests can be very time-consuming). From f57a06aa2bf0e91918ab3429731a44724e5f8c73 Mon Sep 17 00:00:00 2001 From: Emily de la Rua Date: Fri, 4 Oct 2024 17:05:23 -0400 Subject: [PATCH 05/14] Update NEWS --- NEWS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/NEWS.md b/NEWS.md index 06ba194bfe..2bb03424ff 100644 --- a/NEWS.md +++ b/NEWS.md @@ -12,6 +12,7 @@ * Refactored `estimate_incidence_rate` to work as both an analyze function and a summarize function, controlled by the added `summarize` parameter. When `summarize = TRUE`, labels can be fine-tuned via the new `label_fmt` argument to the same function. * Added `fraction` statistic to the `analyze_var_count` method group. * Improved `summarize_glm_count()` documentation and all its associated functions to better describe the results and the functions' purpose. +* Added `method` argument to `s_odds_ratio()` and `estimate_odds_ratio()` to control whether exact or approximate conditional likelihood calculations are used. ### Bug Fixes * Added defaults for `d_count_cumulative` parameters as described in the documentation. From 218b4f62e4e0adc73d25fc11d23d64fb67dd965b Mon Sep 17 00:00:00 2001 From: "27856297+dependabot-preview[bot]@users.noreply.github.com" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Fri, 4 Oct 2024 21:09:06 +0000 Subject: [PATCH 06/14] [skip roxygen] [skip vbump] Roxygen Man Pages Auto Update --- man/try_car_anova.Rd | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/man/try_car_anova.Rd b/man/try_car_anova.Rd index a81cde8287..b3ba87bf83 100644 --- a/man/try_car_anova.Rd +++ b/man/try_car_anova.Rd @@ -9,12 +9,12 @@ try_car_anova(mod, test.statistic) \arguments{ \item{mod}{\code{lm}, \code{aov}, \code{glm}, \code{multinom}, \code{polr} \code{mlm}, \code{coxph}, \code{coxme}, \code{lme}, \code{mer}, \code{merMod}, \code{svyglm}, \code{svycoxph}, - \code{rlm}, or other suitable model object.} + \code{rlm}, \code{clm}, \code{clmm}, or other suitable model object.} \item{test.statistic}{for a generalized linear model, whether to calculate \code{"LR"} (likelihood-ratio), \code{"Wald"}, or \code{"F"} tests; for a Cox or Cox mixed-effects model, whether to calculate \code{"LR"} (partial-likelihood ratio) or - \code{"Wald"} tests; in the default case or for linear mixed models fit by + \code{"Wald"} tests (with \code{"LR"} tests unavailable for Cox models using the \code{tt} argument); in the default case or for linear mixed models fit by \code{lmer}, whether to calculate Wald \code{"Chisq"} or Kenward-Roger \code{"F"} tests with Satterthwaite degrees of freedom (\emph{warning:} the KR F-tests can be very time-consuming). From 491e0bb97df7b628e811a706dfad23f657695718 Mon Sep 17 00:00:00 2001 From: Emily de la Rua Date: Fri, 4 Oct 2024 17:15:46 -0400 Subject: [PATCH 07/14] Empty commit From ec1c689aae7df6580a54c020f1b641ed1e61c396 Mon Sep 17 00:00:00 2001 From: Emily de la Rua Date: Fri, 4 Oct 2024 17:27:03 -0400 Subject: [PATCH 08/14] Remove unused docs --- R/odds_ratio.R | 2 -- 1 file changed, 2 deletions(-) diff --git a/R/odds_ratio.R b/R/odds_ratio.R index 4911a1de61..38a1ab0e68 100644 --- a/R/odds_ratio.R +++ b/R/odds_ratio.R @@ -167,8 +167,6 @@ a_odds_ratio <- make_afun( #' @describeIn odds_ratio Layout-creating function which can take statistics function arguments #' and additional format arguments. This function is a wrapper for [rtables::analyze()]. #' -#' @param ... arguments passed to `s_odds_ratio()`. -#' #' @return #' * `estimate_odds_ratio()` returns a layout object suitable for passing to further layouting functions, #' or to [rtables::build_table()]. Adding this function to an `rtable` layout will add formatted rows containing From 5b470730677dfd4d0a6dcc2ff4b225c5491cb4dc Mon Sep 17 00:00:00 2001 From: "27856297+dependabot-preview[bot]@users.noreply.github.com" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Fri, 4 Oct 2024 21:29:47 +0000 Subject: [PATCH 09/14] [skip roxygen] [skip vbump] Roxygen Man Pages Auto Update --- man/odds_ratio.Rd | 2 -- 1 file changed, 2 deletions(-) diff --git a/man/odds_ratio.Rd b/man/odds_ratio.Rd index 7d25f40ff5..b0361d3410 100644 --- a/man/odds_ratio.Rd +++ b/man/odds_ratio.Rd @@ -98,8 +98,6 @@ by a statistics function.} \item{.in_ref_col}{(\code{flag})\cr \code{TRUE} when working with the reference level, \code{FALSE} otherwise.} \item{.df_row}{(\code{data.frame})\cr data frame across all of the columns for the given row split.} - -\item{...}{arguments passed to \code{s_odds_ratio()}.} } \value{ \itemize{ From 24c72a49736beb130a4a97cd70c8cc69d0dfed71 Mon Sep 17 00:00:00 2001 From: Emily de la Rua Date: Fri, 4 Oct 2024 17:31:13 -0400 Subject: [PATCH 10/14] Empty commit From b46846370171f5adef57fe96937ee23704b3a0c3 Mon Sep 17 00:00:00 2001 From: Emily de la Rua Date: Mon, 7 Oct 2024 15:16:03 -0400 Subject: [PATCH 11/14] Add warning message --- R/odds_ratio.R | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/R/odds_ratio.R b/R/odds_ratio.R index 38a1ab0e68..8203a0595a 100644 --- a/R/odds_ratio.R +++ b/R/odds_ratio.R @@ -130,6 +130,13 @@ s_odds_ratio <- function(df, } } + if (is.na(y$or_ci$est)) { + message( + 'Unable to compute the odds ratio estimate. Please try re-running the function with ', + 'parameter `method` set to "approximate".' + ) + } + y$or_ci <- formatters::with_label( x = y$or_ci, label = paste0("Odds Ratio (", 100 * conf_level, "% CI)") From a32387b44737aae4ee18cc555be51c6266b61d3c Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 7 Oct 2024 19:18:31 +0000 Subject: [PATCH 12/14] [skip style] [skip vbump] Restyle files --- R/odds_ratio.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/odds_ratio.R b/R/odds_ratio.R index 8203a0595a..8e251fa1bf 100644 --- a/R/odds_ratio.R +++ b/R/odds_ratio.R @@ -132,7 +132,7 @@ s_odds_ratio <- function(df, if (is.na(y$or_ci$est)) { message( - 'Unable to compute the odds ratio estimate. Please try re-running the function with ', + "Unable to compute the odds ratio estimate. Please try re-running the function with ", 'parameter `method` set to "approximate".' ) } From 997e33b590023aaa1fd173b8d887b0e6558565a4 Mon Sep 17 00:00:00 2001 From: Emily de la Rua Date: Mon, 7 Oct 2024 15:18:32 -0400 Subject: [PATCH 13/14] Use warning --- R/odds_ratio.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/R/odds_ratio.R b/R/odds_ratio.R index 8203a0595a..2f1e69817f 100644 --- a/R/odds_ratio.R +++ b/R/odds_ratio.R @@ -130,8 +130,8 @@ s_odds_ratio <- function(df, } } - if (is.na(y$or_ci$est)) { - message( + if (is.na(y$or_ci[["est"]]) && method != "approximate") { + warning( 'Unable to compute the odds ratio estimate. Please try re-running the function with ', 'parameter `method` set to "approximate".' ) From aa585557b1cd289389aa72169e1fc0f93a83250a Mon Sep 17 00:00:00 2001 From: Emily de la Rua Date: Mon, 7 Oct 2024 15:26:00 -0400 Subject: [PATCH 14/14] Add test --- R/odds_ratio.R | 4 ++-- tests/testthat/test-odds_ratio.R | 13 +++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/R/odds_ratio.R b/R/odds_ratio.R index 8e251fa1bf..16bb58ac06 100644 --- a/R/odds_ratio.R +++ b/R/odds_ratio.R @@ -130,8 +130,8 @@ s_odds_ratio <- function(df, } } - if (is.na(y$or_ci$est)) { - message( + if ("est" %in% names(y$or_ci) && is.na(y$or_ci[["est"]]) && method != "approximate") { + warning( "Unable to compute the odds ratio estimate. Please try re-running the function with ", 'parameter `method` set to "approximate".' ) diff --git a/tests/testthat/test-odds_ratio.R b/tests/testthat/test-odds_ratio.R index e66535f97f..0a00b93866 100644 --- a/tests/testthat/test-odds_ratio.R +++ b/tests/testthat/test-odds_ratio.R @@ -161,6 +161,7 @@ testthat::test_that("estimate_odds_ratio works with strata and combined groups", }) testthat::test_that("s_odds_ratio method argument works", { + set.seed(1) nex <- 2000 # Number of example rows dta <- data.frame( "rsp" = sample(c(TRUE, FALSE), nex, TRUE), @@ -182,6 +183,18 @@ testthat::test_that("s_odds_ratio method argument works", { ) testthat::expect_false(all(is.na(res$or_ci))) + + # warning works + expect_warning( + s_odds_ratio( + df = subset(dta, grp == "A"), + .var = "rsp", + .ref_group = subset(dta, grp == "B"), + .in_ref_col = FALSE, + .df_row = dta, + variables = list(arm = "grp", strata = "strata") + ) + ) }) testthat::test_that("estimate_odds_ratio method argument works", {