Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor cox regression functions to fix table structure #882

Merged
merged 31 commits into from
Apr 26, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
7efde92
First draft
edelarua Apr 15, 2023
51d7677
2nd iteration
edelarua Apr 18, 2023
8adc7dc
3rd iteration
edelarua Apr 18, 2023
f09678c
Style, lintr
edelarua Apr 18, 2023
4cb8866
4th iteration
edelarua Apr 18, 2023
b03400e
Clean up code
edelarua Apr 18, 2023
053711c
Update documentation for s_coxreg
edelarua Apr 18, 2023
8a68e3f
Fix labels, examples
edelarua Apr 19, 2023
83409e3
Update documentation for summarize_coxreg
edelarua Apr 19, 2023
93cd5f3
Document a_coxreg
edelarua Apr 19, 2023
739417e
Update tests, documentation
edelarua Apr 19, 2023
fc4ddb5
Clean up code
edelarua Apr 20, 2023
a31254c
Merge branch 'main' into 841_cox_refactor@main
edelarua Apr 20, 2023
748565e
Update NEWS
edelarua Apr 20, 2023
ae076c1
Styler, add tests
edelarua Apr 20, 2023
7846845
Merge branch 'main' into 841_cox_refactor@main
edelarua Apr 20, 2023
850ef9c
[skip actions] Roxygen Man Pages Auto Update
dependabot-preview[bot] Apr 20, 2023
0214f0d
Update WORDLIST, roxygen
edelarua Apr 20, 2023
5c2c46b
Fix checks
edelarua Apr 20, 2023
fe68497
Fix internal function examples
edelarua Apr 20, 2023
b2e65b1
Add numeric covariate to multivar tests
edelarua Apr 20, 2023
db40f54
Change .na_str to na_level
edelarua Apr 20, 2023
e9ab4ea
Update to na_level in tests
edelarua Apr 20, 2023
f0d7abd
Merge branch 'main' into 841_cox_refactor@main
shajoezhu Apr 25, 2023
1415333
Add model caching - 7s vs 17s to run tests
edelarua Apr 26, 2023
e380054
Add s_coxreg examples for "multi_lvl" and "inter"
edelarua Apr 26, 2023
7a22212
Fix roxygen lint
edelarua Apr 26, 2023
4b765b4
model_df
Melkiades Apr 26, 2023
bcb4458
fix
Melkiades Apr 26, 2023
7eb52b6
Fix s_coxreg errors
edelarua Apr 26, 2023
9d0e660
Fix roxygen lint
edelarua Apr 26, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 19 additions & 7 deletions R/summarize_coxreg.R
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,8 @@ s_coxreg <- function(df, .stats, .which_vars = "all", .var_nms = NULL) {
#' @param eff (`flag`)\cr whether treatment effect should be calculated. Defaults to `FALSE`.
#' @param var_main (`flag`)\cr whether main effects should be calculated. Defaults to `FALSE`.
#' @param na_level (`string`)\cr custom string to replace all `NA` values with. Defaults to `""`.
#' @param cache_env (`environment`)\cr an environment object used to cache the regression model in order to
#' avoid repeatedly fitting the same model for every row in the table. Defaults to `NULL` (no caching).
#'
#' @examples
#' tern:::a_coxreg(
Expand Down Expand Up @@ -167,7 +169,8 @@ a_coxreg <- function(df,
.spl_context,
.stats,
.formats,
na_level = "") {
na_level = "",
cache_env = NULL) {
cov_no_arm <- !multivar && !"arm" %in% names(variables) && control$interaction # special case: univar no arm
cov <- tail(.spl_context$value, 1) # current variable/covariate
var_lbl <- formatters::var_labels(df)[cov] # check for df labels
Expand All @@ -179,12 +182,17 @@ a_coxreg <- function(df,
if (var_main) control$interaction <- TRUE
}

if (!multivar) {
model <- fit_coxreg_univar(variables = variables, data = df, at = at, control = control) %>% broom::tidy()
if (!var_main) model[, "pval_inter"] <- NA_real_
if (is.null(cache_env[[cov]])) {
if (!multivar) {
model <- fit_coxreg_univar(variables = variables, data = df, at = at, control = control) %>% broom::tidy()
} else {
model <- fit_coxreg_multivar(variables = variables, data = df, control = control) %>% broom::tidy()
}
cache_env[[cov]] <- model
} else {
model <- fit_coxreg_multivar(variables = variables, data = df, control = control) %>% broom::tidy()
model <- cache_env[[cov]]
}
if (!multivar && !var_main) model[, "pval_inter"] <- NA_real_
edelarua marked this conversation as resolved.
Show resolved Hide resolved

if (cov_no_arm) multivar <- TRUE
vars_coxreg <- list(which_vars = "all", var_nms = NULL)
Expand Down Expand Up @@ -309,12 +317,16 @@ summarize_coxreg <- function(lyt,
)
stat_labels <- stat_labels[names(stat_labels) %in% .stats]
.formats <- .formats[names(.formats) %in% .stats]
env <- new.env() # create caching environment

lyt <- lyt %>%
split_cols_by_multivar(
vars = rep(common_var, length(.stats)),
varlabels = stat_labels,
extra_args = list(.stats = .stats, .formats = .formats, na_level = rep(na_level, length(.stats)))
extra_args = list(
.stats = .stats, .formats = .formats, na_level = rep(na_level, length(.stats)),
cache_env = replicate(length(.stats), list(env))
Melkiades marked this conversation as resolved.
Show resolved Hide resolved
)
)

if ("arm" %in% names(variables)) { # treatment effect
Expand Down Expand Up @@ -352,7 +364,7 @@ summarize_coxreg <- function(lyt,
summarize_row_groups(
cfun = a_coxreg,
extra_args = list(
variables = variables, control = control, multivar = multivar,
variables = variables, at = at, control = control, multivar = multivar,
var_main = if (multivar) multivar else control$interaction
)
)
Expand Down
6 changes: 5 additions & 1 deletion man/cox_regression.Rd

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

13 changes: 9 additions & 4 deletions tests/testthat/test-summarize_coxreg.R
Original file line number Diff line number Diff line change
Expand Up @@ -165,12 +165,12 @@ testthat::test_that("summarize_coxreg works without treatment arm in univariable
testthat::test_that("summarize_coxreg adds the multivariable Cox regression layer to rtables", {
variables <- list(time = "TIME", event = "STATUS", arm = "ARMCD", covariates = c("AGE", "COVAR1", "COVAR2"))

lyt <- basic_table() %>%
result <- basic_table() %>%
summarize_coxreg(
variables = variables,
multivar = TRUE
)
result <- lyt %>% build_table(df = dta_bladder)
) %>%
build_table(df = dta_bladder)

res <- testthat::expect_silent(result)
testthat::expect_snapshot(res)
Expand All @@ -189,7 +189,12 @@ testthat::test_that("summarize_coxreg adds the multivariable Cox regression laye

# no labels
formatters::var_labels(dta_bladder) <- rep(NA_character_, ncol(dta_bladder))
result <- lyt %>% build_table(df = dta_bladder)
result <- basic_table() %>%
summarize_coxreg(
variables = variables,
multivar = TRUE
) %>%
build_table(df = dta_bladder)

res <- testthat::expect_silent(result)
testthat::expect_snapshot(res)
Expand Down