From 789d06f071b01846165ecbddd29267458421f0d6 Mon Sep 17 00:00:00 2001 From: Abinaya Yogasekaram <73252787+ayogasekaram@users.noreply.github.com> Date: Tue, 26 Nov 2024 15:15:27 -0500 Subject: [PATCH] update `strata` var to `group_var` (#1246) to match `control_lineplot_vars()` in tern closes #1245 --------- Signed-off-by: Abinaya Yogasekaram <73252787+ayogasekaram@users.noreply.github.com> Co-authored-by: Emily de la Rua <59304861+edelarua@users.noreply.github.com> --- R/tm_g_lineplot.R | 73 +++++++++++++------ man/template_g_lineplot.Rd | 7 +- man/tm_g_lineplot.Rd | 17 +++-- .../testthat/test-shinytest2-tm_g_lineplot.R | 16 ++-- tests/testthat/test-tm_g_lineplot.R | 2 +- 5 files changed, 74 insertions(+), 41 deletions(-) diff --git a/R/tm_g_lineplot.R b/R/tm_g_lineplot.R index ac4b5f5e46..bcaa321eb2 100644 --- a/R/tm_g_lineplot.R +++ b/R/tm_g_lineplot.R @@ -5,7 +5,8 @@ #' @inheritParams tern::g_lineplot #' @inheritParams tern::control_lineplot_vars #' @inheritParams template_arguments -#' @param strata (`string` or `NA`)\cr group variable name. +#' @param strata `r lifecycle::badge("deprecated")` Please use the `group_var` argument instead. +#' @param group_var (`string` or `NA`)\cr group variable name. #' @param param (`character`)\cr parameter to filter the data by. #' @param incl_screen (`logical`)\cr whether the screening visit should be included. #' @param ggplot2_args (`ggplot2_args`) optional\cr object created by [teal.widgets::ggplot2_args()] with settings @@ -22,7 +23,8 @@ #' #' @keywords internal template_g_lineplot <- function(dataname = "ANL", - strata = "ARM", + strata = lifecycle::deprecated(), + group_var = "ARM", x = "AVISIT", y = "AVAL", y_unit = "AVALU", @@ -40,8 +42,17 @@ template_g_lineplot <- function(dataname = "ANL", title = "Line Plot", y_lab = "", ggplot2_args = teal.widgets::ggplot2_args()) { + if (lifecycle::is_present(strata)) { + warning( + "The `strata` argument of `template_g_lineplot()` is deprecated as of teal.modules.clinical 0.9.1. ", + "Please use the `group_var` argument instead.", + call. = FALSE + ) + group_var <- strata + } + checkmate::assert_string(dataname) - checkmate::assert_string(strata) + checkmate::assert_string(group_var) checkmate::assert_string(x) checkmate::assert_string(y) checkmate::assert_string(y_unit) @@ -72,7 +83,7 @@ template_g_lineplot <- function(dataname = "ANL", ) } - # droplevels for strata + # droplevels for group_var data_list <- add_expr( data_list, substitute_names( @@ -80,7 +91,7 @@ template_g_lineplot <- function(dataname = "ANL", arm_var = droplevels(arm_var) ), names = list( - arm_var = as.name(strata) + arm_var = as.name(group_var) ) ) ) @@ -96,7 +107,7 @@ template_g_lineplot <- function(dataname = "ANL", z$variables <- substitute( expr = variables <- control_lineplot_vars(x = x, y = y, group_var = arm, paramcd = paramcd, y_unit = y_unit), - env = list(x = x, y = y, arm = strata, paramcd = paramcd, y_unit = y_unit) + env = list(x = x, y = y, arm = group_var, paramcd = paramcd, y_unit = y_unit) ) mid_choices <- c( @@ -233,7 +244,7 @@ template_g_lineplot <- function(dataname = "ANL", #' tm_g_lineplot( #' label = "Line Plot", #' dataname = "ADLB", -#' strata = choices_selected( +#' group_var = choices_selected( #' variable_choices(ADSL, c("ARM", "ARMCD", "ACTARMCD")), #' "ARM" #' ), @@ -255,12 +266,9 @@ template_g_lineplot <- function(dataname = "ANL", #' @export tm_g_lineplot <- function(label, dataname, - parentname = ifelse( - inherits(strata, "data_extract_spec"), - teal.transform::datanames_input(strata), - "ADSL" - ), - strata = teal.transform::choices_selected( + parentname = NULL, + strata = lifecycle::deprecated(), + group_var = teal.transform::choices_selected( teal.transform::variable_choices(parentname, c("ARM", "ARMCD", "ACTARMCD")), "ARM" ), x = teal.transform::choices_selected( @@ -294,6 +302,25 @@ tm_g_lineplot <- function(label, pre_output = NULL, post_output = NULL, ggplot2_args = teal.widgets::ggplot2_args()) { + if (lifecycle::is_present(strata)) { + warning( + "The `strata` argument of `tm_g_lineplot()` is deprecated as of teal.modules.clinical 0.9.1. ", + "Please use the `group_var` argument instead.", + call. = FALSE + ) + group_var <- strata + } else { + strata <- group_var # resolves missing argument error + } + + # Now handle 'parentname' calculation based on 'group_var' + if (is.null(parentname)) { + parentname <- ifelse( + inherits(group_var, "data_extract_spec"), + teal.transform::datanames_input(group_var), + "ADSL" + ) + } message("Initializing tm_g_lineplot") checkmate::assert_string(label) checkmate::assert_string(dataname) @@ -316,7 +343,7 @@ tm_g_lineplot <- function(label, args <- as.list(environment()) data_extract_list <- list( - strata = cs_to_des_select(strata, dataname = parentname), + group_var = cs_to_des_select(group_var, dataname = parentname), param = cs_to_des_filter(param, dataname = dataname), x = cs_to_des_select(x, dataname = dataname, multiple = FALSE), y = cs_to_des_select(y, dataname = dataname, multiple = FALSE), @@ -348,7 +375,7 @@ tm_g_lineplot <- function(label, ui_g_lineplot <- function(id, ...) { a <- list(...) is_single_dataset_value <- teal.transform::is_single_dataset( - a$strata, + a$group_var, a$paramcd, a$x, a$param, @@ -369,7 +396,7 @@ ui_g_lineplot <- function(id, ...) { teal.reporter::simple_reporter_ui(ns("simple_reporter")), ### tags$label("Encodings", class = "text-primary"), - teal.transform::datanames_input(a[c("strata", "paramcd", "x", "y", "y_unit", "param")]), + teal.transform::datanames_input(a[c("group_var", "paramcd", "x", "y", "y_unit", "param")]), teal.transform::data_extract_ui( id = ns("param"), label = "Select Biomarker", @@ -377,9 +404,9 @@ ui_g_lineplot <- function(id, ...) { is_single_dataset = is_single_dataset_value ), teal.transform::data_extract_ui( - id = ns("strata"), + id = ns("group_var"), label = "Select Treatment Variable", - data_extract_spec = a$strata, + data_extract_spec = a$group_var, is_single_dataset = is_single_dataset_value ), teal.transform::data_extract_ui( @@ -508,7 +535,7 @@ srv_g_lineplot <- function(id, dataname, parentname, paramcd, - strata, + group_var, x, y, param, @@ -525,12 +552,12 @@ srv_g_lineplot <- function(id, moduleServer(id, function(input, output, session) { teal.logger::log_shiny_input_changes(input, namespace = "teal.modules.clinical") selector_list <- teal.transform::data_extract_multiple_srv( - data_extract = list(x = x, y = y, strata = strata, paramcd = paramcd, y_unit = y_unit, param = param), + data_extract = list(x = x, y = y, group_var = group_var, paramcd = paramcd, y_unit = y_unit, param = param), datasets = data, select_validation_rule = list( x = shinyvalidate::sv_required("Please select a time variable"), y = shinyvalidate::sv_required("Please select an analysis variable"), - strata = shinyvalidate::sv_required("Please select a treatment variable") + group_var = shinyvalidate::sv_required("Please select a treatment variable") ), filter_validation_rule = list( param = shinyvalidate::sv_required(message = "Please select Biomarker filter.") @@ -569,7 +596,7 @@ srv_g_lineplot <- function(id, adsl_filtered <- merged$anl_q()[[parentname]] anl_filtered <- merged$anl_q()[[dataname]] - input_strata <- names(merged$anl_input_r()$columns_source$strata) + input_strata <- names(merged$anl_input_r()$columns_source$group_var) input_x_var <- names(merged$anl_input_r()$columns_source$x) input_y <- names(merged$anl_input_r()$columns_source$y) input_param <- unlist(param$filter)["vars_selected"] @@ -612,7 +639,7 @@ srv_g_lineplot <- function(id, my_calls <- template_g_lineplot( dataname = "ANL", - strata = names(merged$anl_input_r()$columns_source$strata), + group_var = names(merged$anl_input_r()$columns_source$group_var), y = names(merged$anl_input_r()$columns_source$y), x = names(merged$anl_input_r()$columns_source$x), paramcd = names(merged$anl_input_r()$columns_source$paramcd), diff --git a/man/template_g_lineplot.Rd b/man/template_g_lineplot.Rd index f60c8b1bf8..f1941082f2 100644 --- a/man/template_g_lineplot.Rd +++ b/man/template_g_lineplot.Rd @@ -6,7 +6,8 @@ \usage{ template_g_lineplot( dataname = "ANL", - strata = "ARM", + strata = lifecycle::deprecated(), + group_var = "ARM", x = "AVISIT", y = "AVAL", y_unit = "AVALU", @@ -29,7 +30,9 @@ template_g_lineplot( \arguments{ \item{dataname}{(\code{character})\cr analysis data used in teal module.} -\item{strata}{(\code{string} or \code{NA})\cr group variable name.} +\item{strata}{\ifelse{html}{\href{https://lifecycle.r-lib.org/articles/stages.html#deprecated}{\figure{lifecycle-deprecated.svg}{options: alt='[Deprecated]'}}}{\strong{[Deprecated]}} Please use the \code{group_var} argument instead.} + +\item{group_var}{(\code{string} or \code{NA})\cr group variable name.} \item{x}{(\code{string})\cr x-variable name.} diff --git a/man/tm_g_lineplot.Rd b/man/tm_g_lineplot.Rd index 0f3dbdf6fc..84504eb8d5 100644 --- a/man/tm_g_lineplot.Rd +++ b/man/tm_g_lineplot.Rd @@ -7,9 +7,10 @@ tm_g_lineplot( label, dataname, - parentname = ifelse(inherits(strata, "data_extract_spec"), - teal.transform::datanames_input(strata), "ADSL"), - strata = teal.transform::choices_selected(teal.transform::variable_choices(parentname, + parentname = NULL, + strata = lifecycle::deprecated(), + group_var = + teal.transform::choices_selected(teal.transform::variable_choices(parentname, c("ARM", "ARMCD", "ACTARMCD")), "ARM"), x = teal.transform::choices_selected(teal.transform::variable_choices(dataname, "AVISIT"), "AVISIT", fixed = TRUE), @@ -44,7 +45,9 @@ tm_g_lineplot( \item{parentname}{(\code{character})\cr parent analysis data used in teal module, usually this refers to \code{ADSL}.} -\item{strata}{(\code{string} or \code{NA})\cr group variable name.} +\item{strata}{\ifelse{html}{\href{https://lifecycle.r-lib.org/articles/stages.html#deprecated}{\figure{lifecycle-deprecated.svg}{options: alt='[Deprecated]'}}}{\strong{[Deprecated]}} Please use the \code{group_var} argument instead.} + +\item{group_var}{(\code{string} or \code{NA})\cr group variable name.} \item{x}{(\code{string})\cr x-variable name.} @@ -132,7 +135,7 @@ app <- init( tm_g_lineplot( label = "Line Plot", dataname = "ADLB", - strata = choices_selected( + group_var = choices_selected( variable_choices(ADSL, c("ARM", "ARMCD", "ACTARMCD")), "ARM" ), @@ -159,8 +162,8 @@ apps implementing this module can be found. \section{Examples in Shinylive}{ \describe{ \item{example-1}{ - \href{https://shinylive.io/r/app/#code=NobwRAdghgtgpmAXGKAHVA6ASmANGAYwHsIAXOMpMAGwEsAjAJykYE8AKcqajGIgEwCu1OAGcMBOhFoFuASgA60snGYFStAG5wABAB4AtDoBmgiOtol2cnQBUsAVQCiSpXSYsOEMaWLUijIoQ7sxs7Pyo1KyBbgyhHMYBsqSiQUr8UKRQ+kZc1AD6GVnW6ZnZhjoA7rSkABa0EOFluDogSjo6AIIAIgDKADI5OqQwBPlwAB75UPyi1O1d3f0AQkMjY5PT-NT0OgCkAHx7Cx0wglnk7J0AagCSvbe2OgC8zybq+YxwAfyqV3cPWwtG73R4AORaMAaciCAF8ggArIgNfIAazgrFETSyNgqv2MUGEpHyBH4tFEYyRKPRmOA0HgWKKUDkAF1XBAegMhkzgMAFGBOf1+Sy2Rylqs8WVefyeithaKlGhUEMGjV2AsmS8dEzcAs+EIRKItfrhGJ1RAOh0RvkAOb5KRwSJEUjmy2W6hQehwaha-n9Bq6AAK-lI-N1Frd2rK9N0bxl4rDJ0tolIzCyWoItWRBDE+VE3rg6jg-FdkY6mhYtE9IhJWZkZsFLQI6oFWAAsmGdDL2wBhbqdmU92ydXv9sAw8NljrdjtgJMdOSTyOsDN1nOiPMFosl+eWiuMKv0GuZ7MN8VNls3TpCvBdsDLTq9Fy3-k9gASAHEB2BA++v+PF13acBWua9+V3QCIzdVAWFgVdTw3fMRG3UsywrahBDgWsEKuc870DEdOjbPtvwIrAiP5SCpzva9bHAqDLSCN0mJ0IIgloYwdHYBpyDUDRtGsGw2gjUR6ggVhOnQdglQAEkEWgWlk-NGG0GIIFhJQwFhFkgA}{Open in Shinylive} - \if{html}{\out{}} + \href{https://shinylive.io/r/app/#code=NobwRAdghgtgpmAXGKAHVA6ASmANGAYwHsIAXOMpMAGwEsAjAJykYE8AKcqajGIgEwCu1OAGcMBOhFoFuASgA60snGYFStAG5wABAB4AtDoBmgiOtol2cnQBUsAVQCiSpXSYsOEMaWLUijIoQ7sxs7Pyo1KyBbgyhHMYBsqSiQUr8UKRQ+kZc1AD6GVnW6ZnZhjoA7rSkABa0EOFluDogSjo6AIIAIgDKADI5OqQwBPlwAB75UPyi1O1d3f0AQkMjY5PT-NT0OgCkAHx7Cx0wglnk7J0AagCSvbe2OgC8zybq+YxwAfyqV3cPWwtG73R4AORaMAaciCAF8ggArIgNfIAazgrFETSyNgqv2MUGEpHyBH4tFEYyRKPRmOA0HgWKKUDkAF1XBAegMhkzgMAFGBOf1+Sy2Rylqs8WVefyeithaKlGhUEMGjV2AsmS8dEzcAs+EIRKItfrhGJ1RAOh0RvkAOb5KRwSJEUjmy2W6hQehwaha-n9Bq6AAK-lI-N1Frd2rK9N0bxl4rDJ0tNsYREEqHymhYWoItWRBDE+VE3rg6jg-FdkY6WcYtE9IhJeZkZsFLQI6oFWAAsmGdDLuwBhbq9mUD2ydQfDsAw8NVjr9ntgJMdOSzyOsHNNguiIslssV5eWmt1+gN3P5lvitsdm6dIV4PtgZadXouB-8gcACQA4iOwIGv1-adV0PecBWuO9+UPECIzdVAWFgTcLx3YsRH3SsqyzahBDgRtkKuK9H0DCdOi7Ic-2IrBSP5GC50fO9bCg2DLSCN1WJ0IIgloYwdHYBpyDUDRtGsGw2gjUR6ggVhOnQdglQAEkEWgWgU4tGG0GIIFhJQwFhFkgA}{Open in Shinylive} + \if{html}{\out{}} \if{html}{\out{}} } } diff --git a/tests/testthat/test-shinytest2-tm_g_lineplot.R b/tests/testthat/test-shinytest2-tm_g_lineplot.R index 35dfe47c77..235210f503 100644 --- a/tests/testthat/test-shinytest2-tm_g_lineplot.R +++ b/tests/testthat/test-shinytest2-tm_g_lineplot.R @@ -16,7 +16,7 @@ app_driver_tm_g_lineplot <- function() { label = "Line Plot", dataname = "ADLB", parentname = "ADSL", - strata = teal.transform::choices_selected( + group_var = teal.transform::choices_selected( teal.transform::variable_choices("ADSL", c("ARM", "ARMCD", "ACTARMCD")), "ARM" ), @@ -76,7 +76,7 @@ testthat::test_that("e2e - tm_g_lineplot: Module initializes in teal without err }) testthat::test_that( - "e2e - tm_g_lineplot: Starts with specified label, param, strata, y, x, mid, interval, incl_screen, + "e2e - tm_g_lineplot: Starts with specified label, param, group_var, y, x, mid, interval, incl_screen, plot_settings and table_settings.", { skip_if_too_deep(5) @@ -85,7 +85,7 @@ testthat::test_that( testthat::expect_equal(trimws(app_driver$get_text("#teal-teal_modules-active_tab > li.active")), "Line Plot") testthat::expect_equal(app_driver$get_active_module_input("param-dataset_ADLB_singleextract-filter1-vals"), "ALT") - testthat::expect_equal(app_driver$get_active_module_input("strata-dataset_ADSL_singleextract-select"), "ARM") + testthat::expect_equal(app_driver$get_active_module_input("group_var-dataset_ADSL_singleextract-select"), "ARM") testthat::expect_equal(app_driver$get_active_module_input("y-dataset_ADLB_singleextract-select"), "AVAL") testthat::expect_equal(app_driver$get_active_module_input("x-dataset_ADLB_singleextract-select"), "AVISIT") testthat::expect_equal(app_driver$get_active_module_input("mid"), "mean") @@ -137,12 +137,12 @@ testthat::test_that("e2e - tm_g_lineplot: Deselecting param throws validation er }) testthat::test_that( - "e2e - tm_g_lineplot: Selecting strata changes plot and doesn't throw validation errors.", + "e2e - tm_g_lineplot: Selecting group_var changes plot and doesn't throw validation errors.", { skip_if_too_deep(5) app_driver <- app_driver_tm_g_lineplot() plot_before <- app_driver$get_active_module_plot_output("myplot") - app_driver$set_active_module_input("strata-dataset_ADSL_singleextract-select", "ARMCD") + app_driver$set_active_module_input("group_var-dataset_ADSL_singleextract-select", "ARMCD") testthat::expect_false( identical( plot_before, @@ -154,14 +154,14 @@ testthat::test_that( } ) -testthat::test_that("e2e - tm_g_lineplot: Deselecting strata throws validation error.", { +testthat::test_that("e2e - tm_g_lineplot: Deselecting group_var throws validation error.", { skip_if_too_deep(5) app_driver <- app_driver_tm_g_lineplot() - app_driver$set_active_module_input("strata-dataset_ADSL_singleextract-select", NULL) + app_driver$set_active_module_input("group_var-dataset_ADSL_singleextract-select", NULL) testthat::expect_identical(app_driver$get_active_module_plot_output("myplot"), character(0)) testthat::expect_identical( app_driver$active_module_element_text( - "strata-dataset_ADSL_singleextract-select_input > div > span" + "group_var-dataset_ADSL_singleextract-select_input > div > span" ), "Please select a treatment variable" ) diff --git a/tests/testthat/test-tm_g_lineplot.R b/tests/testthat/test-tm_g_lineplot.R index b2f805564a..96c6ca8168 100644 --- a/tests/testthat/test-tm_g_lineplot.R +++ b/tests/testthat/test-tm_g_lineplot.R @@ -7,7 +7,7 @@ testthat::test_that("template_g_lineplot works as expected with default argument testthat::test_that("template_g_lineplot gives correct data expression with custom arguments", { result <- template_g_lineplot( - strata = "ARMCD", + group_var = "ARMCD", y = "CHG", mid = "median", interval = "median_ci",