From 1831431e3605c812c33cdc820c20364f2e03688c Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Wed, 27 Sep 2023 17:55:08 +0000 Subject: [PATCH 01/33] feat: #2125 intial draft of new truefalse val args --- R/derive_merged.R | 36 +++++++++++++++++++++-------- R/derive_var_extreme_flag.R | 26 +++++++++++++++++---- man/derive_var_extreme_flag.Rd | 20 ++++++++++++++-- man/derive_vars_merged.Rd | 16 ++++++++++++- tests/testthat/test-derive_merged.R | 2 +- 5 files changed, 83 insertions(+), 17 deletions(-) diff --git a/R/derive_merged.R b/R/derive_merged.R index 428209efaa..53a5ab5830 100644 --- a/R/derive_merged.R +++ b/R/derive_merged.R @@ -82,6 +82,8 @@ #' #' @param match_flag Match flag #' +#' `r lifecycle::badge("deprecated")` Please use `exist_flag` instead. +#' #' If the argument is specified (e.g., `match_flag = FLAG`), the specified #' variable (e.g., `FLAG`) is added to the input dataset. This variable will #' be `TRUE` for all selected records from `dataset_add` which are merged into @@ -89,6 +91,15 @@ #' #' *Permitted Values*: Variable name #' +#' @param exist_flag Exist flag +#' +#' If the argument is specified (e.g., `exist_flag = FLAG`), the specified +#' variable (e.g., `FLAG`) is added to the input dataset. This variable will +#' be `TRUE` for all selected records from `dataset_add` which are merged into +#' the input dataset, and `NA` otherwise. +#' +#' *Permitted Values*: Variable name +#' #' @param missing_values Values for non-matching observations #' #' For observations of the input dataset (`dataset`) which do not have a @@ -199,7 +210,7 @@ #' mode = "last", #' new_vars = exprs(LASTWGT = VSSTRESN, LASTWGTU = VSSTRESU), #' filter_add = VSTESTCD == "WEIGHT", -#' match_flag = vsdatafl +#' exist_flag = vsdatafl #' ) %>% #' select(STUDYID, USUBJID, AGE, AGEU, LASTWGT, LASTWGTU, vsdatafl) #' @@ -284,6 +295,9 @@ derive_vars_merged <- function(dataset, filter_add = NULL, mode = NULL, match_flag = NULL, + exist_flag = NULL, + true_value = TRUE, + false_value = NA, missing_values = NULL, check_type = "warning", duplicate_msg = NULL) { @@ -303,6 +317,9 @@ derive_vars_merged <- function(dataset, ) ) match_flag <- assert_symbol(enexpr(match_flag), optional = TRUE) + exist_flag <- assert_symbol(enexpr(exist_flag), optional = TRUE) + assert_atomic_vector(true_value, optional = TRUE) + assert_atomic_vector(false_value, optional = TRUE) assert_expr_list(missing_values, named = TRUE, optional = TRUE) if (!is.null(missing_values)) { invalid_vars <- setdiff( @@ -349,15 +366,16 @@ derive_vars_merged <- function(dataset, } if (!is.null(missing_values)) { - match_flag_var <- get_new_tmp_var(add_data, prefix = "tmp_match_flag") + exist_flag_var <- get_new_tmp_var(add_data, prefix = "tmp_exist_flag") } else { - match_flag_var <- match_flag + exist_flag_var <- exist_flag } - if (!is.null(match_flag_var)) { + if (!is.null(exist_flag_var)) { add_data <- mutate( add_data, - !!match_flag_var := TRUE + !!exist_flag_var := true_value, + !!exist_flag_var := ifelse(is.na(!!exist_flag_var), false_value, !!exist_flag_var) ) } # check if there are any variables in both datasets which are not by vars @@ -387,7 +405,7 @@ derive_vars_merged <- function(dataset, update_missings <- map2( syms(names(missing_values)), missing_values, - ~ expr(if_else(is.na(!!match_flag_var), !!.y, !!.x)) + ~ expr(if_else(is.na(!!exist_flag_var), !!.y, !!.x)) ) names(update_missings) <- names(missing_values) dataset <- dataset %>% @@ -809,14 +827,14 @@ derive_vars_merged_lookup <- function(dataset, new_vars = new_vars, mode = mode, filter_add = !!filter_add, - match_flag = temp_match_flag, + exist_flag = temp_exist_flag, check_type = check_type, duplicate_msg = duplicate_msg ) if (print_not_mapped) { temp_not_mapped <- res %>% - filter(is.na(temp_match_flag)) %>% + filter(is.na(temp_exist_flag)) %>% distinct(!!!by_vars_left) if (nrow(temp_not_mapped) > 0) { @@ -838,7 +856,7 @@ derive_vars_merged_lookup <- function(dataset, } } - res %>% select(-temp_match_flag) + res %>% select(-temp_exist_flag) } #' Get list of records not mapped from the lookup table. diff --git a/R/derive_var_extreme_flag.R b/R/derive_var_extreme_flag.R index 8cca038477..79b6cc3047 100644 --- a/R/derive_var_extreme_flag.R +++ b/R/derive_var_extreme_flag.R @@ -15,8 +15,8 @@ #' #' @param new_var Variable to add #' -#' The specified variable is added to the output dataset. It is set to `"Y"` -#' for the first or last observation (depending on the mode) of each by group. +#' The specified variable is added to the output dataset. It is set to the value +#' set in `true_value` for the first or last observation (depending on the mode) of each by group. #' #' Permitted Values: list of name-value pairs #' @@ -26,6 +26,20 @@ #' #' Permitted Values: `"first"`, `"last"` #' +#' @param true_value True value +#' +#' The value for the specified variable `new_var`, applicable to +#' the first or last observation (depending on the mode) of each by group. +#' +#' Permitted Values: A character scalar +#' +#' @param false_value False value +#' +#' The value for the specified variable `new_var`, NOT applicable to +#' the first or last observation (depending on the mode) of each by group. +#' +#' Permitted Values: A character scalar +#' #' @param flag_all Flag setting #' #' A logical value where if set to `TRUE`, all records are flagged @@ -233,6 +247,8 @@ derive_var_extreme_flag <- function(dataset, order, new_var, mode, + true_value = "Y", + false_value = NA_character_, flag_all = FALSE, check_type = "warning") { new_var <- assert_symbol(enexpr(new_var)) @@ -240,6 +256,8 @@ derive_var_extreme_flag <- function(dataset, assert_expr_list(order) assert_data_frame(dataset, required_vars = exprs(!!!by_vars, !!!extract_vars(order))) mode <- assert_character_scalar(mode, values = c("first", "last"), case_sensitive = FALSE) + assert_atomic_vector(true_value, optional = TRUE) + assert_atomic_vector(false_value, optional = TRUE) flag_all <- assert_logical_scalar(flag_all) check_type <- assert_character_scalar( check_type, @@ -264,11 +282,11 @@ derive_var_extreme_flag <- function(dataset, if (mode == "first") { data <- data %>% - mutate(!!new_var := if_else(!!tmp_obs_nr == 1, "Y", NA_character_)) + mutate(!!new_var := if_else(!!tmp_obs_nr == 1, true_value, false_value)) } else { data <- data %>% group_by(!!!by_vars) %>% - mutate(!!new_var := if_else(!!tmp_obs_nr == n(), "Y", NA_character_)) %>% + mutate(!!new_var := if_else(!!tmp_obs_nr == n(), true_value, false_value)) %>% ungroup() } diff --git a/man/derive_var_extreme_flag.Rd b/man/derive_var_extreme_flag.Rd index acc55fa615..82ab46976f 100644 --- a/man/derive_var_extreme_flag.Rd +++ b/man/derive_var_extreme_flag.Rd @@ -10,6 +10,8 @@ derive_var_extreme_flag( order, new_var, mode, + true_value = "Y", + false_value = NA_character_, flag_all = FALSE, check_type = "warning" ) @@ -32,8 +34,8 @@ Permitted Values: list of variables or functions of variables} \item{new_var}{Variable to add -The specified variable is added to the output dataset. It is set to \code{"Y"} -for the first or last observation (depending on the mode) of each by group. +The specified variable is added to the output dataset. It is set to the value +set in \code{true_value} for the first or last observation (depending on the mode) of each by group. Permitted Values: list of name-value pairs} @@ -43,6 +45,20 @@ Determines of the first or last observation is flagged. Permitted Values: \code{"first"}, \code{"last"}} +\item{true_value}{True value + +The value for the specified variable \code{new_var}, applicable to +the first or last observation (depending on the mode) of each by group. + +Permitted Values: A character scalar} + +\item{false_value}{False value + +The value for the specified variable \code{new_var}, NOT applicable to +the first or last observation (depending on the mode) of each by group. + +Permitted Values: A character scalar} + \item{flag_all}{Flag setting A logical value where if set to \code{TRUE}, all records are flagged diff --git a/man/derive_vars_merged.Rd b/man/derive_vars_merged.Rd index 78e415ce4d..d68e952b50 100644 --- a/man/derive_vars_merged.Rd +++ b/man/derive_vars_merged.Rd @@ -14,6 +14,9 @@ derive_vars_merged( filter_add = NULL, mode = NULL, match_flag = NULL, + exist_flag = NULL, + true_value = TRUE, + false_value = NA, missing_values = NULL, check_type = "warning", duplicate_msg = NULL @@ -94,6 +97,8 @@ If the \code{order} argument is not specified, the \code{mode} argument is ignor \item{match_flag}{Match flag +\ifelse{html}{\href{https://lifecycle.r-lib.org/articles/stages.html#deprecated}{\figure{lifecycle-deprecated.svg}{options: alt='[Deprecated]'}}}{\strong{[Deprecated]}} Please use \code{exist_flag} instead. + If the argument is specified (e.g., \code{match_flag = FLAG}), the specified variable (e.g., \code{FLAG}) is added to the input dataset. This variable will be \code{TRUE} for all selected records from \code{dataset_add} which are merged into @@ -101,6 +106,15 @@ the input dataset, and \code{NA} otherwise. \emph{Permitted Values}: Variable name} +\item{exist_flag}{Exist flag + +If the argument is specified (e.g., \code{exist_flag = FLAG}), the specified +variable (e.g., \code{FLAG}) is added to the input dataset. This variable will +be \code{TRUE} for all selected records from \code{dataset_add} which are merged into +the input dataset, and \code{NA} otherwise. + +\emph{Permitted Values}: Variable name} + \item{missing_values}{Values for non-matching observations For observations of the input dataset (\code{dataset}) which do not have a @@ -210,7 +224,7 @@ derive_vars_merged( mode = "last", new_vars = exprs(LASTWGT = VSSTRESN, LASTWGTU = VSSTRESU), filter_add = VSTESTCD == "WEIGHT", - match_flag = vsdatafl + exist_flag = vsdatafl ) \%>\% select(STUDYID, USUBJID, AGE, AGEU, LASTWGT, LASTWGTU, vsdatafl) diff --git a/tests/testthat/test-derive_merged.R b/tests/testthat/test-derive_merged.R index ef069a52c8..8330ec64b3 100644 --- a/tests/testthat/test-derive_merged.R +++ b/tests/testthat/test-derive_merged.R @@ -103,7 +103,7 @@ test_that("derive_vars_merged Test 3: merge last value and flag matched by group by_vars = exprs(STUDYID, USUBJID), new_vars = exprs(WEIGHTBL = AVAL), mode = "last", - match_flag = matched + exist_flag = matched ) expected <- adsl %>% mutate( WEIGHTBL = c(68, 88, 55, NA), From 6c2bfea6020a2f5e89ff100cd13567ab28261716 Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Wed, 27 Sep 2023 18:36:50 +0000 Subject: [PATCH 02/33] add word to globals --- R/derive_merged.R | 9 ++++++++- R/globals.R | 1 + 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/R/derive_merged.R b/R/derive_merged.R index 53a5ab5830..4a0fe6ecb7 100644 --- a/R/derive_merged.R +++ b/R/derive_merged.R @@ -316,7 +316,14 @@ derive_vars_merged <- function(dataset, extract_vars(new_vars) ) ) - match_flag <- assert_symbol(enexpr(match_flag), optional = TRUE) + if(!is.null(match_flag)) { + deprecate_warn( + "1.0.0", + "derive_vars_merged(old_param = 'match_flag')", + "derive_vars_merged(new_param = 'exist_flag')" + ) + exist_flag <- assert_symbol(enexpr(match_flag), optional = TRUE) + } exist_flag <- assert_symbol(enexpr(exist_flag), optional = TRUE) assert_atomic_vector(true_value, optional = TRUE) assert_atomic_vector(false_value, optional = TRUE) diff --git a/R/globals.R b/R/globals.R index d1e24ad67a..7fbd1256bc 100644 --- a/R/globals.R +++ b/R/globals.R @@ -105,6 +105,7 @@ globalVariables(c( "temp_from_var", "temp_to_var", "temp_match_flag", + "temp_exist_flag", "temp_dose_freq", "temp_new_dose_no", "temp_num_of_doses", From 8d179bf56c31e363c08ff19a9a61cbdbc37472c3 Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Wed, 27 Sep 2023 18:48:13 +0000 Subject: [PATCH 03/33] chore: #2125 address cicd checks --- R/derive_merged.R | 16 +++++++++++++++- R/derive_var_extreme_flag.R | 4 ++-- man/derive_var_extreme_flag.Rd | 4 ++-- man/derive_vars_merged.Rd | 14 ++++++++++++++ 4 files changed, 33 insertions(+), 5 deletions(-) diff --git a/R/derive_merged.R b/R/derive_merged.R index 4a0fe6ecb7..659f96d6c5 100644 --- a/R/derive_merged.R +++ b/R/derive_merged.R @@ -100,6 +100,20 @@ #' #' *Permitted Values*: Variable name #' +#' @param true_value True value +#' +#' The value for the specified variable `exist_flag`, applicable to +#' the first or last observation (depending on the mode) of each by group. +#' +#' Permitted Values: An atomic scalar +#' +#' @param false_value False value +#' +#' The value for the specified variable `exist_flag`, NOT applicable to +#' the first or last observation (depending on the mode) of each by group. +#' +#' Permitted Values: An atomic scalar +#' #' @param missing_values Values for non-matching observations #' #' For observations of the input dataset (`dataset`) which do not have a @@ -316,7 +330,7 @@ derive_vars_merged <- function(dataset, extract_vars(new_vars) ) ) - if(!is.null(match_flag)) { + if (!is.null(match_flag)) { deprecate_warn( "1.0.0", "derive_vars_merged(old_param = 'match_flag')", diff --git a/R/derive_var_extreme_flag.R b/R/derive_var_extreme_flag.R index 79b6cc3047..0dcab3cdd8 100644 --- a/R/derive_var_extreme_flag.R +++ b/R/derive_var_extreme_flag.R @@ -31,14 +31,14 @@ #' The value for the specified variable `new_var`, applicable to #' the first or last observation (depending on the mode) of each by group. #' -#' Permitted Values: A character scalar +#' Permitted Values: An atomic scalar #' #' @param false_value False value #' #' The value for the specified variable `new_var`, NOT applicable to #' the first or last observation (depending on the mode) of each by group. #' -#' Permitted Values: A character scalar +#' Permitted Values: An atomic scalar #' #' @param flag_all Flag setting #' diff --git a/man/derive_var_extreme_flag.Rd b/man/derive_var_extreme_flag.Rd index 82ab46976f..f96efd75c0 100644 --- a/man/derive_var_extreme_flag.Rd +++ b/man/derive_var_extreme_flag.Rd @@ -50,14 +50,14 @@ Permitted Values: \code{"first"}, \code{"last"}} The value for the specified variable \code{new_var}, applicable to the first or last observation (depending on the mode) of each by group. -Permitted Values: A character scalar} +Permitted Values: An atomic scalar} \item{false_value}{False value The value for the specified variable \code{new_var}, NOT applicable to the first or last observation (depending on the mode) of each by group. -Permitted Values: A character scalar} +Permitted Values: An atomic scalar} \item{flag_all}{Flag setting diff --git a/man/derive_vars_merged.Rd b/man/derive_vars_merged.Rd index d68e952b50..92717a7383 100644 --- a/man/derive_vars_merged.Rd +++ b/man/derive_vars_merged.Rd @@ -115,6 +115,20 @@ the input dataset, and \code{NA} otherwise. \emph{Permitted Values}: Variable name} +\item{true_value}{True value + +The value for the specified variable \code{exist_flag}, applicable to +the first or last observation (depending on the mode) of each by group. + +Permitted Values: An atomic scalar} + +\item{false_value}{False value + +The value for the specified variable \code{exist_flag}, NOT applicable to +the first or last observation (depending on the mode) of each by group. + +Permitted Values: An atomic scalar} + \item{missing_values}{Values for non-matching observations For observations of the input dataset (\code{dataset}) which do not have a From 85d89c08f22e6a8dba6290717eadb4630eb64c54 Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Wed, 27 Sep 2023 19:23:22 +0000 Subject: [PATCH 04/33] feat: #2125 add test for t/f value for derive var extreme flag --- tests/testthat/test-derive_var_extreme_flag.R | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/testthat/test-derive_var_extreme_flag.R b/tests/testthat/test-derive_var_extreme_flag.R index c918c3d146..61062b7191 100644 --- a/tests/testthat/test-derive_var_extreme_flag.R +++ b/tests/testthat/test-derive_var_extreme_flag.R @@ -205,3 +205,34 @@ test_that("derive_var_extreme_flag Test 8: additional case for missing order var keys = c("USUBJID", "AVISITN", "AVAL") ) }) + + +## Test 9: changing true/false flag value works ---- +test_that("derive_var_extreme_flag Test 9: changing true/false flag value works", { + input <- tibble::tribble( + ~USUBJID, ~AVISITN, ~AVAL, + 1, 1, 12, + 1, 3, 9, + 2, 2, 42, + 3, 3, 14, + 3, 3, 10 + ) + + expected_output <- input %>% mutate(firstfl = c("Yes", "No", "Yes", "Yes", "No")) + + actual_output <- derive_var_extreme_flag( + input, + by_vars = exprs(USUBJID), + order = exprs(AVISITN, desc(AVAL)), + new_var = firstfl, + mode = "first", + true_value = "Yes", + false_value = "No" + ) + + expect_dfs_equal( + base = expected_output, + compare = actual_output, + keys = c("USUBJID", "AVISITN", "AVAL") + ) +}) From 92bc048b3ffc5de77db5ae0f3cf33e842ec79213 Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Wed, 27 Sep 2023 19:39:14 +0000 Subject: [PATCH 05/33] feat: #2125 add test for derive_merged truefalse value --- R/derive_merged.R | 6 +- tests/testthat/test-derive_merged.R | 96 ++++++++++++++++++----------- 2 files changed, 63 insertions(+), 39 deletions(-) diff --git a/R/derive_merged.R b/R/derive_merged.R index 659f96d6c5..5019fa7c3a 100644 --- a/R/derive_merged.R +++ b/R/derive_merged.R @@ -395,8 +395,7 @@ derive_vars_merged <- function(dataset, if (!is.null(exist_flag_var)) { add_data <- mutate( add_data, - !!exist_flag_var := true_value, - !!exist_flag_var := ifelse(is.na(!!exist_flag_var), false_value, !!exist_flag_var) + !!exist_flag_var := true_value ) } # check if there are any variables in both datasets which are not by vars @@ -433,7 +432,8 @@ derive_vars_merged <- function(dataset, mutate(!!!update_missings) %>% remove_tmp_vars() } - dataset + dataset %>% + mutate(!!exist_flag_var := ifelse(is.na(!!exist_flag_var), false_value, !!exist_flag_var)) } #' Merge a Categorization Variable diff --git a/tests/testthat/test-derive_merged.R b/tests/testthat/test-derive_merged.R index 8330ec64b3..6277bff7e5 100644 --- a/tests/testthat/test-derive_merged.R +++ b/tests/testthat/test-derive_merged.R @@ -117,8 +117,32 @@ test_that("derive_vars_merged Test 3: merge last value and flag matched by group ) }) -## Test 4: error if variable in both datasets ---- -test_that("derive_vars_merged Test 4: error if variable in both datasets", { +## Test 4: merge last value and flag matched by groups ---- +test_that("derive_vars_merged Test 4: merge last value and flag matched by groups", { + actual <- derive_vars_merged(adsl, + dataset_add = advs, + order = exprs(AVAL), + by_vars = exprs(STUDYID, USUBJID), + new_vars = exprs(WEIGHTBL = AVAL), + mode = "last", + exist_flag = matched, + true_value = "Y", + false_value = "N" + ) + expected <- adsl %>% mutate( + WEIGHTBL = c(68, 88, 55, NA), + matched = c("Y", "Y", "Y", "N") + ) + + expect_dfs_equal( + base = expected, + compare = actual, + keys = c("USUBJID") + ) +}) + +## Test 5: error if variable in both datasets ---- +test_that("derive_vars_merged Test 5: error if variable in both datasets", { expect_error( derive_vars_merged(advs, dataset_add = adsl, @@ -128,8 +152,8 @@ test_that("derive_vars_merged Test 4: error if variable in both datasets", { ) }) -## Test 5: by_vars with rename ---- -test_that("derive_vars_merged Test 5: by_vars with rename", { +## Test 6: by_vars with rename ---- +test_that("derive_vars_merged Test 6: by_vars with rename", { actual <- derive_vars_merged(advs, dataset_add = adsl1, by_vars = exprs(STUDYID, USUBJID = ID), @@ -146,8 +170,8 @@ test_that("derive_vars_merged Test 5: by_vars with rename", { ) }) -## Test 6: expressions for new_vars and missing_values ---- -test_that("derive_vars_merged Test 6: expressions for new_vars and missing_values", { +## Test 7: expressions for new_vars and missing_values ---- +test_that("derive_vars_merged Test 7: expressions for new_vars and missing_values", { actual <- derive_vars_merged( adsl, dataset_add = advs, @@ -169,8 +193,8 @@ test_that("derive_vars_merged Test 6: expressions for new_vars and missing_value ) }) -## Test 7: use new variables in filter_add and order ---- -test_that("derive_vars_merged Test 7: use new variables in filter_add and order", { +## Test 8: use new variables in filter_add and order ---- +test_that("derive_vars_merged Test 8: use new variables in filter_add and order", { expected <- tibble::tribble( ~USUBJID, ~TRTSDT, ~TRTSSEQ, "ST42-1", "2020-12-14", 2, @@ -208,8 +232,8 @@ test_that("derive_vars_merged Test 7: use new variables in filter_add and order" ) }) -## Test 8: warning if not unique w.r.t the by variables and the order ---- -test_that("derive_vars_merged Test 8: warning if not unique w.r.t the by variables and the order", { +## Test 9: warning if not unique w.r.t the by variables and the order ---- +test_that("derive_vars_merged Test 9: warning if not unique w.r.t the by variables and the order", { expect_warning( actual <- derive_vars_merged(advs, dataset_add = adsl2, @@ -222,8 +246,8 @@ test_that("derive_vars_merged Test 8: warning if not unique w.r.t the by variabl ) }) -## Test 9: error if not unique w.r.t the by variables and the order ---- -test_that("derive_vars_merged Test 9: error if not unique w.r.t the by variables and the order", { +## Test 10: error if not unique w.r.t the by variables and the order ---- +test_that("derive_vars_merged Test 10: error if not unique w.r.t the by variables and the order", { expect_error( actual <- derive_vars_merged(advs, dataset_add = adsl2, @@ -237,8 +261,8 @@ test_that("derive_vars_merged Test 9: error if not unique w.r.t the by variables ) }) -## Test 10: error if variables in missing_values but not in new_vars ---- -test_that("derive_vars_merged Test 10: error if variables in missing_values but not in new_vars", { +## Test 11: error if variables in missing_values but not in new_vars ---- +test_that("derive_vars_merged Test 11: error if variables in missing_values but not in new_vars", { expect_error( derive_vars_merged( adsl, @@ -256,8 +280,8 @@ test_that("derive_vars_merged Test 10: error if variables in missing_values but # derive_var_merged_cat ---- -## Test 11: deprecation error ---- -test_that("derive_var_merged_cat Test 11: deprecation error", { +## Test 12: deprecation error ---- +test_that("derive_var_merged_cat Test 12: deprecation error", { get_vscat <- function(x) { if_else(x == "BASELINE", "BASELINE", "POST-BASELINE") } @@ -280,8 +304,8 @@ test_that("derive_var_merged_cat Test 11: deprecation error", { # derive_var_merged_exist_flag ---- -## Test 12: merge existence flag ---- -test_that("derive_var_merged_exist_flag Test 12: merge existence flag", { +## Test 13: merge existence flag ---- +test_that("derive_var_merged_exist_flag Test 13: merge existence flag", { actual <- derive_var_merged_exist_flag( adsl, dataset_add = advs, @@ -301,8 +325,8 @@ test_that("derive_var_merged_exist_flag Test 12: merge existence flag", { ) }) -## Test 13: by_vars with rename ---- -test_that("derive_var_merged_exist_flag Test 13: by_vars with rename", { +## Test 14: by_vars with rename ---- +test_that("derive_var_merged_exist_flag Test 14: by_vars with rename", { actual <- derive_var_merged_exist_flag( adsl, dataset_add = advs1, @@ -324,8 +348,8 @@ test_that("derive_var_merged_exist_flag Test 13: by_vars with rename", { # derive_var_merged_character ---- -## Test 14: deprecation error ---- -test_that("derive_var_merged_character Test 14: deprecation error", { +## Test 15: deprecation error ---- +test_that("derive_var_merged_character Test 15: deprecation error", { expect_error( derive_var_merged_character( adsl, @@ -342,8 +366,8 @@ test_that("derive_var_merged_character Test 14: deprecation error", { # derive_vars_merged_lookup ---- -## Test 15: merge lookup table ---- -test_that("derive_vars_merged_lookup Test 15: merge lookup table", { +## Test 16: merge lookup table ---- +test_that("derive_vars_merged_lookup Test 16: merge lookup table", { param_lookup <- tibble::tribble( ~VSTESTCD, ~VSTEST, ~PARAMCD, ~DESCRIPTION, "WEIGHT", "Weight", "WEIGHT", "Weight (kg)", @@ -380,8 +404,8 @@ test_that("derive_vars_merged_lookup Test 15: merge lookup table", { ## the lookup table -## Test 16: all by_vars have records in the lookup table ---- -test_that("derive_vars_merged_lookup Test 16: all by_vars have records in the lookup table", { +## Test 17: all by_vars have records in the lookup table ---- +test_that("derive_vars_merged_lookup Test 17: all by_vars have records in the lookup table", { param_lookup <- tibble::tribble( ~VSTESTCD, ~VSTEST, ~PARAMCD, ~DESCRIPTION, "WEIGHT", "Weight", "WEIGHT", "Weight (kg)", @@ -416,8 +440,8 @@ test_that("derive_vars_merged_lookup Test 16: all by_vars have records in the l ) }) -## Test 17: by_vars with rename ---- -test_that("derive_vars_merged_lookup Test 17: by_vars with rename", { +## Test 18: by_vars with rename ---- +test_that("derive_vars_merged_lookup Test 18: by_vars with rename", { param_lookup <- tibble::tribble( ~TESTCD, ~VSTEST, ~PARAMCD, ~DESCRIPTION, "WEIGHT", "Weight", "WEIGHT", "Weight (kg)", @@ -453,8 +477,8 @@ test_that("derive_vars_merged_lookup Test 17: by_vars with rename", { # get_not_mapped ---- -## Test 18: not all by_vars have records in the lookup table ---- -test_that("get_not_mapped Test 18: not all by_vars have records in the lookup table", { +## Test 19: not all by_vars have records in the lookup table ---- +test_that("get_not_mapped Test 19: not all by_vars have records in the lookup table", { param_lookup <- tibble::tribble( ~VSTESTCD, ~VSTEST, ~PARAMCD, ~DESCRIPTION, "WEIGHT", "Weight", "WEIGHT", "Weight (kg)", @@ -492,8 +516,8 @@ test_that("get_not_mapped Test 18: not all by_vars have records in the lookup ta }) # derive_var_merged_summary ---- -## Test 19: dataset == dataset_add, no filter ---- -test_that("derive_var_merged_summary Test 19: dataset == dataset_add, no filter", { +## Test 20: dataset == dataset_add, no filter ---- +test_that("derive_var_merged_summary Test 20: dataset == dataset_add, no filter", { expected <- tibble::tribble( ~AVISIT, ~ASEQ, ~AVAL, ~MEANVIS, "WEEK 1", 1, 10, 10, @@ -521,8 +545,8 @@ test_that("derive_var_merged_summary Test 19: dataset == dataset_add, no filter" ) }) -## Test 20: dataset != dataset_add, filter ---- -test_that("derive_var_merged_summary Test 20: dataset != dataset_add, filter", { +## Test 21: dataset != dataset_add, filter ---- +test_that("derive_var_merged_summary Test 21: dataset != dataset_add, filter", { expected <- tibble::tribble( ~USUBJID, ~MEANPBL, "1", 13.5, @@ -555,8 +579,8 @@ test_that("derive_var_merged_summary Test 20: dataset != dataset_add, filter", { ) }) -## Test 21: by_vars with rename ---- -test_that("derive_var_merged_summary Test 21: by_vars with rename", { +## Test 22: by_vars with rename ---- +test_that("derive_var_merged_summary Test 22: by_vars with rename", { expected <- tibble::tribble( ~AVISIT, ~ASEQ, ~AVAL, ~MEANVIS, "WEEK 1", 1, 10, 10, From b8e4f03a3320fea76094350360f4ad5321298186 Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Wed, 27 Sep 2023 19:45:24 +0000 Subject: [PATCH 06/33] chore: #2125 address rcmd error --- R/derive_merged.R | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/R/derive_merged.R b/R/derive_merged.R index 5019fa7c3a..73c90b6664 100644 --- a/R/derive_merged.R +++ b/R/derive_merged.R @@ -432,8 +432,11 @@ derive_vars_merged <- function(dataset, mutate(!!!update_missings) %>% remove_tmp_vars() } - dataset %>% - mutate(!!exist_flag_var := ifelse(is.na(!!exist_flag_var), false_value, !!exist_flag_var)) + if (!is.null(exist_flag_var)) { + dataset <- dataset %>% + mutate(!!exist_flag_var := ifelse(is.na(!!exist_flag_var), false_value, !!exist_flag_var)) + } + dataset } #' Merge a Categorization Variable From 179283d0b004f46283d0e0dbd06244ca84be0a21 Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Thu, 28 Sep 2023 15:35:01 +0000 Subject: [PATCH 07/33] address errors --- R/derive_merged.R | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/R/derive_merged.R b/R/derive_merged.R index 73c90b6664..e12db9e2e6 100644 --- a/R/derive_merged.R +++ b/R/derive_merged.R @@ -429,14 +429,14 @@ derive_vars_merged <- function(dataset, ) names(update_missings) <- names(missing_values) dataset <- dataset %>% - mutate(!!!update_missings) %>% - remove_tmp_vars() + mutate(!!!update_missings) } if (!is.null(exist_flag_var)) { dataset <- dataset %>% mutate(!!exist_flag_var := ifelse(is.na(!!exist_flag_var), false_value, !!exist_flag_var)) } - dataset + dataset %>% + remove_tmp_vars() } #' Merge a Categorization Variable From 48861aee98641c5434f71caf56721749b8d740cd Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Thu, 28 Sep 2023 15:36:55 +0000 Subject: [PATCH 08/33] address styler --- tests/testthat/test-derive_merged.R | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/testthat/test-derive_merged.R b/tests/testthat/test-derive_merged.R index 6277bff7e5..6760b1e090 100644 --- a/tests/testthat/test-derive_merged.R +++ b/tests/testthat/test-derive_merged.R @@ -120,14 +120,14 @@ test_that("derive_vars_merged Test 3: merge last value and flag matched by group ## Test 4: merge last value and flag matched by groups ---- test_that("derive_vars_merged Test 4: merge last value and flag matched by groups", { actual <- derive_vars_merged(adsl, - dataset_add = advs, - order = exprs(AVAL), - by_vars = exprs(STUDYID, USUBJID), - new_vars = exprs(WEIGHTBL = AVAL), - mode = "last", - exist_flag = matched, - true_value = "Y", - false_value = "N" + dataset_add = advs, + order = exprs(AVAL), + by_vars = exprs(STUDYID, USUBJID), + new_vars = exprs(WEIGHTBL = AVAL), + mode = "last", + exist_flag = matched, + true_value = "Y", + false_value = "N" ) expected <- adsl %>% mutate( WEIGHTBL = c(68, 88, 55, NA), From d56b2352c155ef1516330478bf7cd7ff77f1b347 Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Thu, 28 Sep 2023 16:52:48 +0000 Subject: [PATCH 09/33] feat: #2125 change default true/false to "Y"/NA --- R/derive_merged.R | 4 ++-- man/derive_vars_merged.Rd | 4 ++-- tests/testthat/test-derive_merged.R | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/R/derive_merged.R b/R/derive_merged.R index e12db9e2e6..c90e261845 100644 --- a/R/derive_merged.R +++ b/R/derive_merged.R @@ -310,8 +310,8 @@ derive_vars_merged <- function(dataset, mode = NULL, match_flag = NULL, exist_flag = NULL, - true_value = TRUE, - false_value = NA, + true_value = "Y", + false_value = NA_character_, missing_values = NULL, check_type = "warning", duplicate_msg = NULL) { diff --git a/man/derive_vars_merged.Rd b/man/derive_vars_merged.Rd index 92717a7383..528ae29337 100644 --- a/man/derive_vars_merged.Rd +++ b/man/derive_vars_merged.Rd @@ -15,8 +15,8 @@ derive_vars_merged( mode = NULL, match_flag = NULL, exist_flag = NULL, - true_value = TRUE, - false_value = NA, + true_value = "Y", + false_value = NA_character_, missing_values = NULL, check_type = "warning", duplicate_msg = NULL diff --git a/tests/testthat/test-derive_merged.R b/tests/testthat/test-derive_merged.R index 6760b1e090..9b6295eab4 100644 --- a/tests/testthat/test-derive_merged.R +++ b/tests/testthat/test-derive_merged.R @@ -107,7 +107,7 @@ test_that("derive_vars_merged Test 3: merge last value and flag matched by group ) expected <- adsl %>% mutate( WEIGHTBL = c(68, 88, 55, NA), - matched = c(TRUE, TRUE, TRUE, NA) + matched = c("Y", "Y", "Y", NA_character_) ) expect_dfs_equal( From de1306efb9c7e092cb57a741b97b82589388bb8f Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Thu, 28 Sep 2023 17:06:43 +0000 Subject: [PATCH 10/33] chore: #2125 add news blurb --- NEWS.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/NEWS.md b/NEWS.md index 9f48559ee0..2009bdafd9 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,20 @@ +# admiral (development version) + +## New Features + +## Updates of Existing Functions + +- `derive_var_extreme_flag()` and `derive_vars_merged()` were enhanced with the arguments `true_value` and `false_value` to align with preexisting functions that had similar functionality (#2125) + +## Breaking Changes + +- For the function `derive_vars_merged()`, the argument `match_flag` was renamed to `exist_flag` (#2125) + +## Documentation + +## Various + + # admiral 0.12.1 - `derive_extreme_records()` no longer fails if `dataset_add` is specified and a From 2d70d69c4a62814c0706c716b63d62ed92b495cb Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Fri, 6 Oct 2023 18:20:06 +0000 Subject: [PATCH 11/33] address merge conflicts --- tests/testthat/test-derive_merged.R | 52 ----------------------------- 1 file changed, 52 deletions(-) diff --git a/tests/testthat/test-derive_merged.R b/tests/testthat/test-derive_merged.R index c0e9ef1b60..fb62c08bf7 100644 --- a/tests/testthat/test-derive_merged.R +++ b/tests/testthat/test-derive_merged.R @@ -278,40 +278,9 @@ test_that("derive_vars_merged Test 11: error if variables in missing_values but ) }) -<<<<<<< HEAD -# derive_var_merged_cat ---- - -## Test 12: deprecation error ---- -test_that("derive_var_merged_cat Test 12: deprecation error", { - get_vscat <- function(x) { - if_else(x == "BASELINE", "BASELINE", "POST-BASELINE") - } - - expect_error( - derive_var_merged_cat( - adsl, - dataset_add = advs, - by_vars = exprs(USUBJID), - new_var = LSTVSCAT, - source_var = AVISIT, - cat_fun = get_vscat, - order = exprs(AVISIT), - mode = "last", - missing_value = "MISSING" - ), - class = "lifecycle_error_deprecated" - ) -}) - - -# derive_var_merged_exist_flag ---- -## Test 13: merge existence flag ---- -test_that("derive_var_merged_exist_flag Test 13: merge existence flag", { -======= # derive_var_merged_exist_flag ---- ## Test 11: merge existence flag ---- test_that("derive_var_merged_exist_flag Test 11: merge existence flag", { ->>>>>>> main actual <- derive_var_merged_exist_flag( adsl, dataset_add = advs, @@ -331,13 +300,8 @@ test_that("derive_var_merged_exist_flag Test 11: merge existence flag", { ) }) -<<<<<<< HEAD -## Test 14: by_vars with rename ---- -test_that("derive_var_merged_exist_flag Test 14: by_vars with rename", { -======= ## Test 12: by_vars with rename ---- test_that("derive_var_merged_exist_flag Test 12: by_vars with rename", { ->>>>>>> main actual <- derive_var_merged_exist_flag( adsl, dataset_add = advs1, @@ -357,7 +321,6 @@ test_that("derive_var_merged_exist_flag Test 12: by_vars with rename", { ) }) -<<<<<<< HEAD # derive_var_merged_character ---- ## Test 15: deprecation error ---- @@ -377,14 +340,9 @@ test_that("derive_var_merged_character Test 15: deprecation error", { }) -# derive_vars_merged_lookup ---- -## Test 16: merge lookup table ---- -test_that("derive_vars_merged_lookup Test 16: merge lookup table", { -======= # derive_vars_merged_lookup ---- ## Test 13: merge lookup table ---- test_that("derive_vars_merged_lookup Test 13: merge lookup table", { ->>>>>>> main param_lookup <- tibble::tribble( ~VSTESTCD, ~VSTEST, ~PARAMCD, ~DESCRIPTION, "WEIGHT", "Weight", "WEIGHT", "Weight (kg)", @@ -421,13 +379,8 @@ test_that("derive_vars_merged_lookup Test 13: merge lookup table", { ## the lookup table -<<<<<<< HEAD -## Test 17: all by_vars have records in the lookup table ---- -test_that("derive_vars_merged_lookup Test 17: all by_vars have records in the lookup table", { -======= ## Test 14: all by_vars have records in the lookup table ---- test_that("derive_vars_merged_lookup Test 14: all by_vars have records in the lookup table", { ->>>>>>> main param_lookup <- tibble::tribble( ~VSTESTCD, ~VSTEST, ~PARAMCD, ~DESCRIPTION, "WEIGHT", "Weight", "WEIGHT", "Weight (kg)", @@ -462,13 +415,8 @@ test_that("derive_vars_merged_lookup Test 14: all by_vars have records in the l ) }) -<<<<<<< HEAD -## Test 18: by_vars with rename ---- -test_that("derive_vars_merged_lookup Test 18: by_vars with rename", { -======= ## Test 15: by_vars with rename ---- test_that("derive_vars_merged_lookup Test 15: by_vars with rename", { ->>>>>>> main param_lookup <- tibble::tribble( ~TESTCD, ~VSTEST, ~PARAMCD, ~DESCRIPTION, "WEIGHT", "Weight", "WEIGHT", "Weight (kg)", From 8d27e82608ccdfec288f6206f7e4c1b63ffa6de3 Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Fri, 6 Oct 2023 18:29:57 +0000 Subject: [PATCH 12/33] forgot to remove a snippet --- tests/testthat/test-derive_merged.R | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/tests/testthat/test-derive_merged.R b/tests/testthat/test-derive_merged.R index fb62c08bf7..1e3c921f7f 100644 --- a/tests/testthat/test-derive_merged.R +++ b/tests/testthat/test-derive_merged.R @@ -321,25 +321,6 @@ test_that("derive_var_merged_exist_flag Test 12: by_vars with rename", { ) }) -# derive_var_merged_character ---- - -## Test 15: deprecation error ---- -test_that("derive_var_merged_character Test 15: deprecation error", { - expect_error( - derive_var_merged_character( - adsl, - dataset_add = advs, - by_vars = exprs(USUBJID), - order = exprs(AVISIT), - new_var = LASTVIS, - source_var = AVISIT, - mode = "last" - ), - class = "lifecycle_error_deprecated" - ) -}) - - # derive_vars_merged_lookup ---- ## Test 13: merge lookup table ---- test_that("derive_vars_merged_lookup Test 13: merge lookup table", { From 0274688303f5318d297ac6d7dcc22eb614232d2d Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Wed, 11 Oct 2023 17:21:11 +0000 Subject: [PATCH 13/33] feat: #2125 add T/F values for derive_vars_joined as well --- R/derive_joined.R | 6 ++++++ man/derive_vars_joined.Rd | 26 ++++++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/R/derive_joined.R b/R/derive_joined.R index 586f256360..7f8267d940 100644 --- a/R/derive_joined.R +++ b/R/derive_joined.R @@ -329,6 +329,9 @@ derive_vars_joined <- function(dataset, filter_add = NULL, filter_join = NULL, mode = NULL, + exist_flag = NULL, + true_value = "Y", + false_value = NA_character_, missing_values = NULL, check_type = "warning") { assert_vars(by_vars, optional = TRUE) @@ -423,6 +426,9 @@ derive_vars_joined <- function(dataset, new_vars = add_suffix_to_vars(new_vars, vars = common_vars, suffix = ".join"), missing_values = missing_values, check_type = check_type, + exist_flag = exist_flag, + true_value = true_value, + false_value = false_value, duplicate_msg = paste( paste( "After applying `filter_join` the joined dataset contains more", diff --git a/man/derive_vars_joined.Rd b/man/derive_vars_joined.Rd index 1b3452d615..e4d5d136d3 100644 --- a/man/derive_vars_joined.Rd +++ b/man/derive_vars_joined.Rd @@ -15,6 +15,9 @@ derive_vars_joined( filter_add = NULL, filter_join = NULL, mode = NULL, + exist_flag = NULL, + true_value = "Y", + false_value = NA_character_, missing_values = NULL, check_type = "warning" ) @@ -122,6 +125,29 @@ If the \code{order} argument is not specified, the \code{mode} argument is ignor \emph{Permitted Values}: \code{"first"}, \code{"last"}, \code{NULL}} +\item{exist_flag}{Exist flag + +If the argument is specified (e.g., \code{exist_flag = FLAG}), the specified +variable (e.g., \code{FLAG}) is added to the input dataset. This variable will +be \code{TRUE} for all selected records from \code{dataset_add} which are merged into +the input dataset, and \code{NA} otherwise. + +\emph{Permitted Values}: Variable name} + +\item{true_value}{True value + +The value for the specified variable \code{exist_flag}, applicable to +the first or last observation (depending on the mode) of each by group. + +Permitted Values: An atomic scalar} + +\item{false_value}{False value + +The value for the specified variable \code{exist_flag}, NOT applicable to +the first or last observation (depending on the mode) of each by group. + +Permitted Values: An atomic scalar} + \item{missing_values}{Values for non-matching observations For observations of the input dataset (\code{dataset}) which do not have a From 466cd053c61a2a70fb7d2c19352d9a51114ecd96 Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Wed, 11 Oct 2023 17:38:17 +0000 Subject: [PATCH 14/33] feat: #2125 align derive_extreme_records too and fix test suite --- R/derive_extreme_records.R | 2 +- R/derive_joined.R | 1 - man/derive_extreme_records.Rd | 2 +- tests/testthat/test-derive_extreme_records.R | 4 ++-- 4 files changed, 4 insertions(+), 5 deletions(-) diff --git a/R/derive_extreme_records.R b/R/derive_extreme_records.R index 981e3d96ea..f63698d17e 100644 --- a/R/derive_extreme_records.R +++ b/R/derive_extreme_records.R @@ -263,7 +263,7 @@ derive_extreme_records <- function(dataset = NULL, check_type = "warning", exist_flag = NULL, true_value = "Y", - false_value = "N", + false_value = NA_character_, keep_source_vars = exprs(everything()), set_values_to) { # Check input arguments diff --git a/R/derive_joined.R b/R/derive_joined.R index 7f8267d940..9eabec30e6 100644 --- a/R/derive_joined.R +++ b/R/derive_joined.R @@ -426,7 +426,6 @@ derive_vars_joined <- function(dataset, new_vars = add_suffix_to_vars(new_vars, vars = common_vars, suffix = ".join"), missing_values = missing_values, check_type = check_type, - exist_flag = exist_flag, true_value = true_value, false_value = false_value, duplicate_msg = paste( diff --git a/man/derive_extreme_records.Rd b/man/derive_extreme_records.Rd index 3063a5056f..a394c9b104 100644 --- a/man/derive_extreme_records.Rd +++ b/man/derive_extreme_records.Rd @@ -15,7 +15,7 @@ derive_extreme_records( check_type = "warning", exist_flag = NULL, true_value = "Y", - false_value = "N", + false_value = NA_character_, keep_source_vars = exprs(everything()), set_values_to ) diff --git a/tests/testthat/test-derive_extreme_records.R b/tests/testthat/test-derive_extreme_records.R index b3531cbad4..53d34bc804 100644 --- a/tests/testthat/test-derive_extreme_records.R +++ b/tests/testthat/test-derive_extreme_records.R @@ -89,9 +89,9 @@ test_that("derive_extreme_records Test 2: derive first PD date", { adrs, tibble::tribble( ~USUBJID, ~ADT, ~AVALC, - "1", ymd(""), "N", + "1", ymd(""), NA_character_, "2", ymd("2021-07-16"), "Y", - "3", ymd(""), "N" + "3", ymd(""), NA_character_ ) %>% mutate( STUDYID = "XX1234", From 15ea0f060ebd6d2b91ccef7534dc35cdc5d6469c Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Wed, 11 Oct 2023 17:59:16 +0000 Subject: [PATCH 15/33] did that fix it --- R/derive_joined.R | 1 + 1 file changed, 1 insertion(+) diff --git a/R/derive_joined.R b/R/derive_joined.R index 9eabec30e6..7f8267d940 100644 --- a/R/derive_joined.R +++ b/R/derive_joined.R @@ -426,6 +426,7 @@ derive_vars_joined <- function(dataset, new_vars = add_suffix_to_vars(new_vars, vars = common_vars, suffix = ".join"), missing_values = missing_values, check_type = check_type, + exist_flag = exist_flag, true_value = true_value, false_value = false_value, duplicate_msg = paste( From 729bef861260c17680d952221b23aa0de63b8895 Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Wed, 11 Oct 2023 18:16:16 +0000 Subject: [PATCH 16/33] implications of this --- R/derive_joined.R | 1 - 1 file changed, 1 deletion(-) diff --git a/R/derive_joined.R b/R/derive_joined.R index 7f8267d940..9eabec30e6 100644 --- a/R/derive_joined.R +++ b/R/derive_joined.R @@ -426,7 +426,6 @@ derive_vars_joined <- function(dataset, new_vars = add_suffix_to_vars(new_vars, vars = common_vars, suffix = ".join"), missing_values = missing_values, check_type = check_type, - exist_flag = exist_flag, true_value = true_value, false_value = false_value, duplicate_msg = paste( From d8cfbf49fe7fb67a5a10b30b192478e67edbf94c Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Wed, 11 Oct 2023 19:17:12 +0000 Subject: [PATCH 17/33] feat: #2125 add back exist_flag toggle --- R/derive_joined.R | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/R/derive_joined.R b/R/derive_joined.R index 9eabec30e6..f3f5f32f3d 100644 --- a/R/derive_joined.R +++ b/R/derive_joined.R @@ -419,13 +419,14 @@ derive_vars_joined <- function(dataset, } # merge new variables to the input dataset and rename them - data %>% + data_final <- data %>% derive_vars_merged( dataset_add = data_return, by_vars = exprs(!!!by_vars_left, !!tmp_obs_nr), new_vars = add_suffix_to_vars(new_vars, vars = common_vars, suffix = ".join"), missing_values = missing_values, check_type = check_type, + exist_flag = exist_flag, true_value = true_value, false_value = false_value, duplicate_msg = paste( @@ -441,4 +442,9 @@ derive_vars_joined <- function(dataset, ) ) %>% remove_tmp_vars() + if (is.null(exist_flag)) { + data_final <- data_final %>% + select(-starts_with("exist_")) + } + data_final } From bd50f2267b3b9e85cff42d112b156b018e0a372f Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Wed, 11 Oct 2023 20:10:25 +0000 Subject: [PATCH 18/33] update news blurb --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 9657f8de0c..f552c544af 100644 --- a/NEWS.md +++ b/NEWS.md @@ -4,7 +4,7 @@ ## Updates of Existing Functions -- `derive_var_extreme_flag()` and `derive_vars_merged()` were enhanced with the arguments `true_value` and `false_value` to align with preexisting functions that had similar functionality (#2125) +- `derive_extreme_records()`, `derive_var_extreme_flag()`,`derive_vars_joined()` and `derive_vars_merged()` were enhanced with the arguments `true_value` and `false_value` to align with preexisting functions that had similar functionality (#2125) - `restrict_derivation()` now allows `{dplyr}` functions like `mutate` in the `derivation argument (#2143) From 1ada1e6957b1a1f0de8e15fe4610e97802e6b1b9 Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Thu, 12 Oct 2023 15:01:48 +0000 Subject: [PATCH 19/33] fix the rlang stuff --- R/derive_joined.R | 7 ++---- tests/testthat/test-derive_joined.R | 34 +++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/R/derive_joined.R b/R/derive_joined.R index f3f5f32f3d..bee05be985 100644 --- a/R/derive_joined.R +++ b/R/derive_joined.R @@ -426,7 +426,7 @@ derive_vars_joined <- function(dataset, new_vars = add_suffix_to_vars(new_vars, vars = common_vars, suffix = ".join"), missing_values = missing_values, check_type = check_type, - exist_flag = exist_flag, + exist_flag = !!enexpr(exist_flag), true_value = true_value, false_value = false_value, duplicate_msg = paste( @@ -442,9 +442,6 @@ derive_vars_joined <- function(dataset, ) ) %>% remove_tmp_vars() - if (is.null(exist_flag)) { - data_final <- data_final %>% - select(-starts_with("exist_")) - } + data_final } diff --git a/tests/testthat/test-derive_joined.R b/tests/testthat/test-derive_joined.R index eeb1afcc23..f52587cef7 100644 --- a/tests/testthat/test-derive_joined.R +++ b/tests/testthat/test-derive_joined.R @@ -333,3 +333,37 @@ test_that("derive_vars_joined Test 10: order vars are selected properly in funct keys = c("day", "val", "first_val") ) }) + +## Test 11: Ensure exist_flag, true/false value arguments work ---- +test_that("derive_vars_joined Test 11: Ensure exist_flag, true/false value arguments work", { + expected <- tibble::tribble( + ~USUBJID, ~ADY, ~AVISIT, ~AWLO, ~AWHI, ~flag, + "1", -2, "BASELINE", -30, 1, "Yes", + "1", 3, "WEEK 1", 2, 7, "Yes", + "1", 24, "WEEK 4", 23, 30, "Yes", + "2", NA, NA, NA, NA, "No" + ) + + windows <- tibble::tribble( + ~AVISIT, ~AWLO, ~AWHI, + "BASELINE", -30, 1, + "WEEK 1", 2, 7, + "WEEK 2", 8, 15, + "WEEK 3", 16, 22, + "WEEK 4", 23, 30 + ) + + expect_dfs_equal( + base = expected, + comp = derive_vars_joined( + select(expected, USUBJID, ADY), + dataset_add = windows, + join_vars = exprs(AWHI, AWLO), + filter_join = AWLO <= ADY & ADY <= AWHI, + exist_flag = flag, + true_value = "Yes", + false_value = "No" + ), + keys = c("USUBJID", "ADY") + ) +}) From c5a6ae2a2629385437337663f45340e5623ccf2f Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Thu, 12 Oct 2023 15:02:50 +0000 Subject: [PATCH 20/33] less code modification --- R/derive_joined.R | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/R/derive_joined.R b/R/derive_joined.R index bee05be985..f166e05031 100644 --- a/R/derive_joined.R +++ b/R/derive_joined.R @@ -419,7 +419,7 @@ derive_vars_joined <- function(dataset, } # merge new variables to the input dataset and rename them - data_final <- data %>% + data %>% derive_vars_merged( dataset_add = data_return, by_vars = exprs(!!!by_vars_left, !!tmp_obs_nr), @@ -442,6 +442,4 @@ derive_vars_joined <- function(dataset, ) ) %>% remove_tmp_vars() - - data_final } From 18d4119164203da14d14b97393fcf9c071bc45f4 Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Thu, 12 Oct 2023 16:44:01 +0000 Subject: [PATCH 21/33] add news blurb --- NEWS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS.md b/NEWS.md index f552c544af..e329cf950f 100644 --- a/NEWS.md +++ b/NEWS.md @@ -6,6 +6,8 @@ - `derive_extreme_records()`, `derive_var_extreme_flag()`,`derive_vars_joined()` and `derive_vars_merged()` were enhanced with the arguments `true_value` and `false_value` to align with preexisting functions that had similar functionality (#2125) +- The default value for the `false_value` argument in `derive_extreme_records()` was changed to `NA_character_` (#2125) + - `restrict_derivation()` now allows `{dplyr}` functions like `mutate` in the `derivation argument (#2143) - `derive_summary_records()`, `derive_var_merged_summary()`, and `get_summary_records()` From f34ae890afeffb4773000db27442e4147f3942dc Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Thu, 12 Oct 2023 16:51:51 +0000 Subject: [PATCH 22/33] fix documentation --- R/derive_merged.R | 4 ++-- man/derive_vars_joined.Rd | 4 ++-- man/derive_vars_merged.Rd | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/R/derive_merged.R b/R/derive_merged.R index aa36bbe947..7fbe55ab61 100644 --- a/R/derive_merged.R +++ b/R/derive_merged.R @@ -94,8 +94,8 @@ #' #' If the argument is specified (e.g., `exist_flag = FLAG`), the specified #' variable (e.g., `FLAG`) is added to the input dataset. This variable will -#' be `TRUE` for all selected records from `dataset_add` which are merged into -#' the input dataset, and `NA` otherwise. +#' be the value provided in `true_value` for all selected records from `dataset_add` +#' which are merged into the input dataset, and the value provided in `false_value` otherwise. #' #' *Permitted Values*: Variable name #' diff --git a/man/derive_vars_joined.Rd b/man/derive_vars_joined.Rd index e4d5d136d3..5092e89ab8 100644 --- a/man/derive_vars_joined.Rd +++ b/man/derive_vars_joined.Rd @@ -129,8 +129,8 @@ If the \code{order} argument is not specified, the \code{mode} argument is ignor If the argument is specified (e.g., \code{exist_flag = FLAG}), the specified variable (e.g., \code{FLAG}) is added to the input dataset. This variable will -be \code{TRUE} for all selected records from \code{dataset_add} which are merged into -the input dataset, and \code{NA} otherwise. +be the value provided in \code{true_value} for all selected records from \code{dataset_add} +which are merged into the input dataset, and the value provided in \code{false_value} otherwise. \emph{Permitted Values}: Variable name} diff --git a/man/derive_vars_merged.Rd b/man/derive_vars_merged.Rd index 2e04872e3c..d198cd0ca8 100644 --- a/man/derive_vars_merged.Rd +++ b/man/derive_vars_merged.Rd @@ -110,8 +110,8 @@ the input dataset, and \code{NA} otherwise. If the argument is specified (e.g., \code{exist_flag = FLAG}), the specified variable (e.g., \code{FLAG}) is added to the input dataset. This variable will -be \code{TRUE} for all selected records from \code{dataset_add} which are merged into -the input dataset, and \code{NA} otherwise. +be the value provided in \code{true_value} for all selected records from \code{dataset_add} +which are merged into the input dataset, and the value provided in \code{false_value} otherwise. \emph{Permitted Values}: Variable name} From 13ea0a6ed450cd78eba850532c1c1076883c0bf8 Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Thu, 12 Oct 2023 17:30:00 +0000 Subject: [PATCH 23/33] feat: #2125 separate exist_flag and missing_value functionality --- R/derive_merged.R | 28 ++++++---- tests/testthat/test-derive_merged.R | 83 +++++++++++++++++++---------- 2 files changed, 75 insertions(+), 36 deletions(-) diff --git a/R/derive_merged.R b/R/derive_merged.R index 7fbe55ab61..5a767b83a4 100644 --- a/R/derive_merged.R +++ b/R/derive_merged.R @@ -385,18 +385,26 @@ derive_vars_merged <- function(dataset, select(!!!by_vars_right, !!!replace_values_by_names(new_vars)) } + if (!is.null(exist_flag)) { + add_data <- mutate( + add_data, + !!exist_flag := TRUE + ) + } + if (!is.null(missing_values)) { - exist_flag_var <- get_new_tmp_var(add_data, prefix = "tmp_exist_flag") + missing_values_var <- get_new_tmp_var(add_data, prefix = "tmp_missing_flag") } else { - exist_flag_var <- exist_flag + missing_values_var <- missing_values } - - if (!is.null(exist_flag_var)) { + if (!is.null(missing_values_var)) { add_data <- mutate( add_data, - !!exist_flag_var := true_value + !!missing_values_var := TRUE ) } + + # check if there are any variables in both datasets which are not by vars # in this case an error is issued to avoid renaming of varibles by left_join() common_vars <- @@ -420,20 +428,22 @@ derive_vars_merged <- function(dataset, } dataset <- left_join(dataset, add_data, by = vars2chr(by_vars)) - if (!is.null(missing_values)) { + if (!is.null(missing_values_var)) { update_missings <- map2( syms(names(missing_values)), missing_values, - ~ expr(if_else(is.na(!!exist_flag_var), !!.y, !!.x)) + ~ expr(if_else(is.na(!!missing_values_var), !!.y, !!.x)) ) names(update_missings) <- names(missing_values) dataset <- dataset %>% mutate(!!!update_missings) } - if (!is.null(exist_flag_var)) { + + if (!is.null(exist_flag)) { dataset <- dataset %>% - mutate(!!exist_flag_var := ifelse(is.na(!!exist_flag_var), false_value, !!exist_flag_var)) + mutate(!!exist_flag := ifelse(is.na(!!exist_flag), false_value, true_value)) } + dataset %>% remove_tmp_vars() } diff --git a/tests/testthat/test-derive_merged.R b/tests/testthat/test-derive_merged.R index 337a2fc3ef..fe2ab77dac 100644 --- a/tests/testthat/test-derive_merged.R +++ b/tests/testthat/test-derive_merged.R @@ -193,8 +193,36 @@ test_that("derive_vars_merged Test 7: expressions for new_vars and missing_value ) }) -## Test 8: use new variables in filter_add and order ---- -test_that("derive_vars_merged Test 8: use new variables in filter_add and order", { + +## Test 8: expressions for new_vars and missing_values and exist_flags ---- +test_that("derive_vars_merged Test 8: expressions for new_vars and missing_values and exist_flags", { + actual <- derive_vars_merged( + adsl, + dataset_add = advs, + by_vars = exprs(USUBJID), + order = exprs(AVISIT), + new_vars = exprs(LASTVIS = str_to_upper(AVISIT)), + mode = "last", + missing_values = exprs(LASTVIS = "UNKNOWN"), + exist_flag = matched, + true_value = "Yes", + false_value = "No" + ) + + expected <- adsl %>% + mutate(LASTVIS = c("WEEK 2", "BASELINE", "WEEK 4", "UNKNOWN"), + matched = c("Yes", "Yes", "Yes", "No")) + + + expect_dfs_equal( + base = expected, + compare = actual, + keys = "USUBJID" + ) +}) + +## Test 9: use new variables in filter_add and order ---- +test_that("derive_vars_merged Test 9: use new variables in filter_add and order", { expected <- tibble::tribble( ~USUBJID, ~TRTSDT, ~TRTSSEQ, "ST42-1", "2020-12-14", 2, @@ -232,8 +260,8 @@ test_that("derive_vars_merged Test 8: use new variables in filter_add and order" ) }) -## Test 9: warning if not unique w.r.t the by variables and the order ---- -test_that("derive_vars_merged Test 9: warning if not unique w.r.t the by variables and the order", { +## Test 10: warning if not unique w.r.t the by variables and the order ---- +test_that("derive_vars_merged Test 10: warning if not unique w.r.t the by variables and the order", { expect_warning( actual <- derive_vars_merged(advs, dataset_add = adsl2, @@ -246,8 +274,8 @@ test_that("derive_vars_merged Test 9: warning if not unique w.r.t the by variabl ) }) -## Test 10: error if not unique w.r.t the by variables and the order ---- -test_that("derive_vars_merged Test 10: error if not unique w.r.t the by variables and the order", { +## Test 11: error if not unique w.r.t the by variables and the order ---- +test_that("derive_vars_merged Test 11: error if not unique w.r.t the by variables and the order", { expect_error( actual <- derive_vars_merged(advs, dataset_add = adsl2, @@ -261,8 +289,8 @@ test_that("derive_vars_merged Test 10: error if not unique w.r.t the by variable ) }) -## Test 11: error if variables in missing_values but not in new_vars ---- -test_that("derive_vars_merged Test 11: error if variables in missing_values but not in new_vars", { +## Test 12: error if variables in missing_values but not in new_vars ---- +test_that("derive_vars_merged Test 12: error if variables in missing_values but not in new_vars", { expect_error( derive_vars_merged( adsl, @@ -279,8 +307,8 @@ test_that("derive_vars_merged Test 11: error if variables in missing_values but }) # derive_var_merged_exist_flag ---- -## Test 11: merge existence flag ---- -test_that("derive_var_merged_exist_flag Test 11: merge existence flag", { +## Test 13: merge existence flag ---- +test_that("derive_var_merged_exist_flag Test 13: merge existence flag", { actual <- derive_var_merged_exist_flag( adsl, dataset_add = advs, @@ -300,8 +328,8 @@ test_that("derive_var_merged_exist_flag Test 11: merge existence flag", { ) }) -## Test 12: by_vars with rename ---- -test_that("derive_var_merged_exist_flag Test 12: by_vars with rename", { +## Test 14: by_vars with rename ---- +test_that("derive_var_merged_exist_flag Test 14: by_vars with rename", { actual <- derive_var_merged_exist_flag( adsl, dataset_add = advs1, @@ -322,8 +350,8 @@ test_that("derive_var_merged_exist_flag Test 12: by_vars with rename", { }) # derive_vars_merged_lookup ---- -## Test 13: merge lookup table ---- -test_that("derive_vars_merged_lookup Test 13: merge lookup table", { +## Test 15: merge lookup table ---- +test_that("derive_vars_merged_lookup Test 15: merge lookup table", { param_lookup <- tibble::tribble( ~VSTESTCD, ~VSTEST, ~PARAMCD, ~DESCRIPTION, "WEIGHT", "Weight", "WEIGHT", "Weight (kg)", @@ -360,8 +388,8 @@ test_that("derive_vars_merged_lookup Test 13: merge lookup table", { ## the lookup table -## Test 14: all by_vars have records in the lookup table ---- -test_that("derive_vars_merged_lookup Test 14: all by_vars have records in the lookup table", { +## Test 16: all by_vars have records in the lookup table ---- +test_that("derive_vars_merged_lookup Test 16: all by_vars have records in the lookup table", { param_lookup <- tibble::tribble( ~VSTESTCD, ~VSTEST, ~PARAMCD, ~DESCRIPTION, "WEIGHT", "Weight", "WEIGHT", "Weight (kg)", @@ -396,8 +424,8 @@ test_that("derive_vars_merged_lookup Test 14: all by_vars have records in the l ) }) -## Test 15: by_vars with rename ---- -test_that("derive_vars_merged_lookup Test 15: by_vars with rename", { +## Test 17: by_vars with rename ---- +test_that("derive_vars_merged_lookup Test 17: by_vars with rename", { param_lookup <- tibble::tribble( ~TESTCD, ~VSTEST, ~PARAMCD, ~DESCRIPTION, "WEIGHT", "Weight", "WEIGHT", "Weight (kg)", @@ -433,8 +461,8 @@ test_that("derive_vars_merged_lookup Test 15: by_vars with rename", { # get_not_mapped ---- -## Test 16: not all by_vars have records in the lookup table ---- -test_that("get_not_mapped Test 16: not all by_vars have records in the lookup table", { +## Test 18: not all by_vars have records in the lookup table ---- +test_that("get_not_mapped Test 18: not all by_vars have records in the lookup table", { param_lookup <- tibble::tribble( ~VSTESTCD, ~VSTEST, ~PARAMCD, ~DESCRIPTION, "WEIGHT", "Weight", "WEIGHT", "Weight (kg)", @@ -472,8 +500,8 @@ test_that("get_not_mapped Test 16: not all by_vars have records in the lookup ta }) # derive_var_merged_summary ---- -## Test 17: dataset == dataset_add, no filter ---- -test_that("derive_var_merged_summary Test 17: dataset == dataset_add, no filter", { +## Test 19: dataset == dataset_add, no filter ---- +test_that("derive_var_merged_summary Test 19: dataset == dataset_add, no filter", { expected <- tibble::tribble( ~AVISIT, ~ASEQ, ~AVAL, ~MEANVIS, "WEEK 1", 1, 10, 10, @@ -499,8 +527,8 @@ test_that("derive_var_merged_summary Test 17: dataset == dataset_add, no filter" ) }) -## Test 18: dataset != dataset_add, filter ---- -test_that("derive_var_merged_summary Test 18: dataset != dataset_add, filter", { +## Test 20: dataset != dataset_add, filter ---- +test_that("derive_var_merged_summary Test 20: dataset != dataset_add, filter", { expected <- tibble::tribble( ~USUBJID, ~MEANPBL, "1", 13.5, @@ -531,8 +559,8 @@ test_that("derive_var_merged_summary Test 18: dataset != dataset_add, filter", { ) }) -## Test 19: by_vars with rename ---- -test_that("derive_var_merged_summary Test 19: by_vars with rename", { +## Test 21: by_vars with rename ---- +test_that("derive_var_merged_summary Test 21: by_vars with rename", { expected <- tibble::tribble( ~AVISIT, ~ASEQ, ~AVAL, ~MEANVIS, "WEEK 1", 1, 10, 10, @@ -559,7 +587,8 @@ test_that("derive_var_merged_summary Test 19: by_vars with rename", { ) }) -test_that("derive_var_merged_summary Test 19: deprecation warning", { +## Test 22: deprecation warning ---- +test_that("derive_var_merged_summary Test 22: deprecation warning", { expected <- tibble::tribble( ~AVISIT, ~ASEQ, ~AVAL, ~MEANVIS, "WEEK 1", 1, 10, 10, From 9b4172fb72053af7feac53a5d06a7d78031c081d Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Thu, 12 Oct 2023 17:34:58 +0000 Subject: [PATCH 24/33] super edge case to account for in feedback --- tests/testthat/test-derive_merged.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/testthat/test-derive_merged.R b/tests/testthat/test-derive_merged.R index fe2ab77dac..51ffe33d8a 100644 --- a/tests/testthat/test-derive_merged.R +++ b/tests/testthat/test-derive_merged.R @@ -205,13 +205,13 @@ test_that("derive_vars_merged Test 8: expressions for new_vars and missing_value mode = "last", missing_values = exprs(LASTVIS = "UNKNOWN"), exist_flag = matched, - true_value = "Yes", + true_value = NA, false_value = "No" ) expected <- adsl %>% mutate(LASTVIS = c("WEEK 2", "BASELINE", "WEEK 4", "UNKNOWN"), - matched = c("Yes", "Yes", "Yes", "No")) + matched = c(NA, NA, NA, "No")) expect_dfs_equal( From d7d2f0c9294196462a677d258109a7ed70e350a8 Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Thu, 12 Oct 2023 17:38:04 +0000 Subject: [PATCH 25/33] revert renaming of the temp flag for lookup --- R/derive_merged.R | 6 +++--- R/globals.R | 1 - 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/R/derive_merged.R b/R/derive_merged.R index 5a767b83a4..0bd9a2a3f1 100644 --- a/R/derive_merged.R +++ b/R/derive_merged.R @@ -711,14 +711,14 @@ derive_vars_merged_lookup <- function(dataset, new_vars = new_vars, mode = mode, filter_add = !!filter_add, - exist_flag = temp_exist_flag, + exist_flag = temp_match_flag, check_type = check_type, duplicate_msg = duplicate_msg ) if (print_not_mapped) { temp_not_mapped <- res %>% - filter(is.na(temp_exist_flag)) %>% + filter(is.na(temp_match_flag)) %>% distinct(!!!by_vars_left) if (nrow(temp_not_mapped) > 0) { @@ -740,7 +740,7 @@ derive_vars_merged_lookup <- function(dataset, } } - res %>% select(-temp_exist_flag) + res %>% select(-temp_match_flag) } #' Get list of records not mapped from the lookup table. diff --git a/R/globals.R b/R/globals.R index 7fbd1256bc..d1e24ad67a 100644 --- a/R/globals.R +++ b/R/globals.R @@ -105,7 +105,6 @@ globalVariables(c( "temp_from_var", "temp_to_var", "temp_match_flag", - "temp_exist_flag", "temp_dose_freq", "temp_new_dose_no", "temp_num_of_doses", From b384bcc83e4eac1a5e97443332d704a326be3a21 Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Thu, 12 Oct 2023 17:42:47 +0000 Subject: [PATCH 26/33] run styler --- tests/testthat/test-derive_merged.R | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/testthat/test-derive_merged.R b/tests/testthat/test-derive_merged.R index 51ffe33d8a..31d68faf69 100644 --- a/tests/testthat/test-derive_merged.R +++ b/tests/testthat/test-derive_merged.R @@ -210,8 +210,10 @@ test_that("derive_vars_merged Test 8: expressions for new_vars and missing_value ) expected <- adsl %>% - mutate(LASTVIS = c("WEEK 2", "BASELINE", "WEEK 4", "UNKNOWN"), - matched = c(NA, NA, NA, "No")) + mutate( + LASTVIS = c("WEEK 2", "BASELINE", "WEEK 4", "UNKNOWN"), + matched = c(NA, NA, NA, "No") + ) expect_dfs_equal( From 92aaf07a5810cc3eaf3b7bad92723b8282fbaad3 Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Thu, 12 Oct 2023 17:47:47 +0000 Subject: [PATCH 27/33] cleaner code changes --- R/derive_merged.R | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/R/derive_merged.R b/R/derive_merged.R index 0bd9a2a3f1..e734ed497a 100644 --- a/R/derive_merged.R +++ b/R/derive_merged.R @@ -436,7 +436,8 @@ derive_vars_merged <- function(dataset, ) names(update_missings) <- names(missing_values) dataset <- dataset %>% - mutate(!!!update_missings) + mutate(!!!update_missings) %>% + remove_tmp_vars() } if (!is.null(exist_flag)) { @@ -444,8 +445,7 @@ derive_vars_merged <- function(dataset, mutate(!!exist_flag := ifelse(is.na(!!exist_flag), false_value, true_value)) } - dataset %>% - remove_tmp_vars() + dataset } #' Merge an Existence Flag From 368f4af0dedef8c76a73345bb278e54ab26cef52 Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Thu, 12 Oct 2023 20:08:06 +0000 Subject: [PATCH 28/33] lintr --- tests/testthat/test-derive_merged.R | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/testthat/test-derive_merged.R b/tests/testthat/test-derive_merged.R index 31d68faf69..646103c1f2 100644 --- a/tests/testthat/test-derive_merged.R +++ b/tests/testthat/test-derive_merged.R @@ -194,8 +194,8 @@ test_that("derive_vars_merged Test 7: expressions for new_vars and missing_value }) -## Test 8: expressions for new_vars and missing_values and exist_flags ---- -test_that("derive_vars_merged Test 8: expressions for new_vars and missing_values and exist_flags", { +## Test 8: Use of missing_values and exist_flags ---- +test_that("derive_vars_merged Test 8: Use of missing_values and exist_flags", { actual <- derive_vars_merged( adsl, dataset_add = advs, @@ -263,7 +263,7 @@ test_that("derive_vars_merged Test 9: use new variables in filter_add and order" }) ## Test 10: warning if not unique w.r.t the by variables and the order ---- -test_that("derive_vars_merged Test 10: warning if not unique w.r.t the by variables and the order", { +test_that("derive_vars_merged Test 10: warning if not unique w.r.t the by variables and the order", { # nolint expect_warning( actual <- derive_vars_merged(advs, dataset_add = adsl2, From 90a77e3f41b25f477a9cf463149a8baa1c0e5bd4 Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Fri, 13 Oct 2023 14:02:09 +0000 Subject: [PATCH 29/33] chore: #2125 change news blurb and fix deprecation messaging --- NEWS.md | 4 ++-- R/derive_merged.R | 8 ++++---- man/derive_vars_merged.Rd | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/NEWS.md b/NEWS.md index e329cf950f..cdb5e9b036 100644 --- a/NEWS.md +++ b/NEWS.md @@ -6,8 +6,6 @@ - `derive_extreme_records()`, `derive_var_extreme_flag()`,`derive_vars_joined()` and `derive_vars_merged()` were enhanced with the arguments `true_value` and `false_value` to align with preexisting functions that had similar functionality (#2125) -- The default value for the `false_value` argument in `derive_extreme_records()` was changed to `NA_character_` (#2125) - - `restrict_derivation()` now allows `{dplyr}` functions like `mutate` in the `derivation argument (#2143) - `derive_summary_records()`, `derive_var_merged_summary()`, and `get_summary_records()` @@ -28,6 +26,8 @@ were enhanced such that more than one summary variable can be derived, e.g., - For the function `derive_vars_merged()`, the argument `match_flag` was renamed to `exist_flag` (#2125) +- The default value for the `false_value` argument in `derive_extreme_records()` was changed to `NA_character_` (#2125) + - The following functions, which were deprecated in previous `{admiral}` versions, have been removed: (#2098) - `derive_param_extreme_event()` - `derive_vars_last_dose()` diff --git a/R/derive_merged.R b/R/derive_merged.R index e734ed497a..df19e4b97f 100644 --- a/R/derive_merged.R +++ b/R/derive_merged.R @@ -307,7 +307,7 @@ derive_vars_merged <- function(dataset, new_vars = NULL, filter_add = NULL, mode = NULL, - match_flag = NULL, + match_flag, exist_flag = NULL, true_value = "Y", false_value = NA_character_, @@ -329,11 +329,11 @@ derive_vars_merged <- function(dataset, extract_vars(new_vars) ) ) - if (!is.null(match_flag)) { + if (!is_missing(match_flag)) { deprecate_warn( "1.0.0", - "derive_vars_merged(old_param = 'match_flag')", - "derive_vars_merged(new_param = 'exist_flag')" + "derive_vars_merged(match_flag =)", + "derive_vars_merged(exist_flag =)" ) exist_flag <- assert_symbol(enexpr(match_flag), optional = TRUE) } diff --git a/man/derive_vars_merged.Rd b/man/derive_vars_merged.Rd index d198cd0ca8..dca45a6222 100644 --- a/man/derive_vars_merged.Rd +++ b/man/derive_vars_merged.Rd @@ -13,7 +13,7 @@ derive_vars_merged( new_vars = NULL, filter_add = NULL, mode = NULL, - match_flag = NULL, + match_flag, exist_flag = NULL, true_value = "Y", false_value = NA_character_, From e3453e7632d1ac8225b0ffed7c67edcf0b9fd5f2 Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Fri, 13 Oct 2023 14:11:19 +0000 Subject: [PATCH 30/33] cleaner to read --- R/derive_merged.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/derive_merged.R b/R/derive_merged.R index df19e4b97f..53f3c32575 100644 --- a/R/derive_merged.R +++ b/R/derive_merged.R @@ -395,7 +395,7 @@ derive_vars_merged <- function(dataset, if (!is.null(missing_values)) { missing_values_var <- get_new_tmp_var(add_data, prefix = "tmp_missing_flag") } else { - missing_values_var <- missing_values + missing_values_var <- NULL } if (!is.null(missing_values_var)) { add_data <- mutate( From 95e398dfc06a56a89808d0a0940c0977fe9f7432 Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Fri, 13 Oct 2023 14:26:52 +0000 Subject: [PATCH 31/33] chore: #2125 use tmp var for derive_vars_merged_lookup --- R/derive_merged.R | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/R/derive_merged.R b/R/derive_merged.R index 53f3c32575..e49d7cea67 100644 --- a/R/derive_merged.R +++ b/R/derive_merged.R @@ -703,6 +703,8 @@ derive_vars_merged_lookup <- function(dataset, assert_logical_scalar(print_not_mapped) filter_add <- assert_filter_cond(enexpr(filter_add), optional = TRUE) + tmp_lookup_flag <- get_new_tmp_var(dataset_add, prefix = "tmp_lookup_flag") + res <- derive_vars_merged( dataset, dataset_add, @@ -711,14 +713,14 @@ derive_vars_merged_lookup <- function(dataset, new_vars = new_vars, mode = mode, filter_add = !!filter_add, - exist_flag = temp_match_flag, + exist_flag = !!tmp_lookup_flag, check_type = check_type, duplicate_msg = duplicate_msg ) if (print_not_mapped) { temp_not_mapped <- res %>% - filter(is.na(temp_match_flag)) %>% + filter(is.na(!!tmp_lookup_flag)) %>% distinct(!!!by_vars_left) if (nrow(temp_not_mapped) > 0) { @@ -740,7 +742,7 @@ derive_vars_merged_lookup <- function(dataset, } } - res %>% select(-temp_match_flag) + res %>% remove_tmp_vars() } #' Get list of records not mapped from the lookup table. From 458096ba799e9abf373d784726f759c0afbf416a Mon Sep 17 00:00:00 2001 From: "Bundfuss, Stefan {MDBB~Basel}" Date: Fri, 13 Oct 2023 16:56:52 +0200 Subject: [PATCH 32/33] #2125 add_truefalse_value_flags: simplify code --- R/derive_merged.R | 30 +++++++++++------------------- 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/R/derive_merged.R b/R/derive_merged.R index e49d7cea67..27ab7a0f2d 100644 --- a/R/derive_merged.R +++ b/R/derive_merged.R @@ -385,26 +385,18 @@ derive_vars_merged <- function(dataset, select(!!!by_vars_right, !!!replace_values_by_names(new_vars)) } - if (!is.null(exist_flag)) { - add_data <- mutate( - add_data, - !!exist_flag := TRUE - ) - } - - if (!is.null(missing_values)) { - missing_values_var <- get_new_tmp_var(add_data, prefix = "tmp_missing_flag") + if (!is.null(missing_values) || !is.null(exist_flag)) { + match_flag_var <- get_new_tmp_var(add_data, prefix = "tmp_match_flag") } else { - missing_values_var <- NULL + match_flag_var <- NULL } - if (!is.null(missing_values_var)) { + if (!is.null(match_flag_var)) { add_data <- mutate( add_data, - !!missing_values_var := TRUE + !!match_flag_var := TRUE ) } - # check if there are any variables in both datasets which are not by vars # in this case an error is issued to avoid renaming of varibles by left_join() common_vars <- @@ -428,24 +420,24 @@ derive_vars_merged <- function(dataset, } dataset <- left_join(dataset, add_data, by = vars2chr(by_vars)) - if (!is.null(missing_values_var)) { + if (!is.null(match_flag_var)) { update_missings <- map2( syms(names(missing_values)), missing_values, - ~ expr(if_else(is.na(!!missing_values_var), !!.y, !!.x)) + ~ expr(if_else(is.na(!!match_flag_var), !!.y, !!.x)) ) names(update_missings) <- names(missing_values) dataset <- dataset %>% - mutate(!!!update_missings) %>% - remove_tmp_vars() + mutate(!!!update_missings) } if (!is.null(exist_flag)) { dataset <- dataset %>% - mutate(!!exist_flag := ifelse(is.na(!!exist_flag), false_value, true_value)) + mutate(!!exist_flag := ifelse(is.na(!!match_flag_var), false_value, true_value)) } - dataset + dataset %>% + remove_tmp_vars() } #' Merge an Existence Flag From e2fb3aec8c65987d4f77a8bc763f1c06996dc99b Mon Sep 17 00:00:00 2001 From: Zelos Zhu Date: Fri, 13 Oct 2023 15:50:56 +0000 Subject: [PATCH 33/33] feat: #2125 styler & add test for deprecation warning --- R/derive_merged.R | 4 ++-- R/globals.R | 1 - tests/testthat/test-derive_merged.R | 14 ++++++++++++++ 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/R/derive_merged.R b/R/derive_merged.R index 27ab7a0f2d..0bd7e9b279 100644 --- a/R/derive_merged.R +++ b/R/derive_merged.R @@ -329,7 +329,7 @@ derive_vars_merged <- function(dataset, extract_vars(new_vars) ) ) - if (!is_missing(match_flag)) { + if (!is_missing(enexpr(match_flag))) { deprecate_warn( "1.0.0", "derive_vars_merged(match_flag =)", @@ -695,7 +695,7 @@ derive_vars_merged_lookup <- function(dataset, assert_logical_scalar(print_not_mapped) filter_add <- assert_filter_cond(enexpr(filter_add), optional = TRUE) - tmp_lookup_flag <- get_new_tmp_var(dataset_add, prefix = "tmp_lookup_flag") + tmp_lookup_flag <- get_new_tmp_var(dataset_add, prefix = "tmp_lookup_flag") res <- derive_vars_merged( dataset, diff --git a/R/globals.R b/R/globals.R index d1e24ad67a..7a4855e8d2 100644 --- a/R/globals.R +++ b/R/globals.R @@ -104,7 +104,6 @@ globalVariables(c( "temp_DT", "temp_from_var", "temp_to_var", - "temp_match_flag", "temp_dose_freq", "temp_new_dose_no", "temp_num_of_doses", diff --git a/tests/testthat/test-derive_merged.R b/tests/testthat/test-derive_merged.R index 646103c1f2..c9bf4d8ad4 100644 --- a/tests/testthat/test-derive_merged.R +++ b/tests/testthat/test-derive_merged.R @@ -308,6 +308,20 @@ test_that("derive_vars_merged Test 12: error if variables in missing_values but ) }) +test_that("deprecation messaging for match_flag", { + expect_warning( + derive_vars_merged(adsl, + dataset_add = advs, + order = exprs(AVAL), + by_vars = exprs(STUDYID, USUBJID), + new_vars = exprs(WEIGHTBL = AVAL), + mode = "last", + match_flag = matched + ), + class = "lifecycle_warning_deprecated" + ) +}) + # derive_var_merged_exist_flag ---- ## Test 13: merge existence flag ---- test_that("derive_var_merged_exist_flag Test 13: merge existence flag", {