-
Notifications
You must be signed in to change notification settings - Fork 68
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
closes #2481 bug the result of derive param tte depends on the sort order of the input #2569
base: main
Are you sure you want to change the base?
Changes from all commits
8e23388
cd52801
2727736
9e86217
d97377c
fa49a51
01e8f5a
53457c2
020c9d7
dccdbe1
8006891
4c95243
087c0f3
4405868
21b5a00
ce07ad1
1d4e6b7
22f3f2d
47637a5
e882758
e5c28fc
404c949
ae70492
34d2fb3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -60,6 +60,14 @@ | |||||||||||
#' | ||||||||||||
#' A list of symbols created using `exprs()` is expected. | ||||||||||||
#' | ||||||||||||
#' @param check_type Check uniqueness | ||||||||||||
#' | ||||||||||||
#' If `"warning"`, `"message"`, or `"error"` is specified, the specified message is issued | ||||||||||||
#' if the observations of the input dataset are not unique with respect to the | ||||||||||||
#' by variables and the order. | ||||||||||||
Comment on lines
+66
to
+67
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||
#' | ||||||||||||
#' Default: `"none"` | ||||||||||||
ProfessorP-beep marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||
#' | ||||||||||||
#' @details The following steps are performed to create the observations of the | ||||||||||||
#' new parameter: | ||||||||||||
#' | ||||||||||||
|
@@ -262,10 +270,10 @@ | |||||||||||
#' mutate(STUDYID = "AB42") | ||||||||||||
#' | ||||||||||||
#' ae <- tribble( | ||||||||||||
#' ~USUBJID, ~AESTDTC, ~AESEQ, ~AEDECOD, | ||||||||||||
#' "01", "2021-01-03T10:56", 1, "Flu", | ||||||||||||
#' "01", "2021-03-04", 2, "Cough", | ||||||||||||
#' "01", "2021", 3, "Flu" | ||||||||||||
#' ~USUBJID, ~AESTDTC, ~AESEQ, ~AEDECOD, | ||||||||||||
#' "01", "2021-01-03T10:56", 1, "Flu", | ||||||||||||
#' "01", "2021-03-04", 2, "Cough", | ||||||||||||
#' "01", "2021-", 3, "Flu" | ||||||||||||
Comment on lines
-265
to
+276
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why has this been changed? Now the example generates a warning. |
||||||||||||
#' ) %>% | ||||||||||||
#' mutate(STUDYID = "AB42") | ||||||||||||
#' | ||||||||||||
|
@@ -313,6 +321,50 @@ | |||||||||||
#' ) | ||||||||||||
#' ) %>% | ||||||||||||
#' select(USUBJID, STARTDT, PARAMCD, PARAM, ADT, CNSR, SRCSEQ) | ||||||||||||
#' | ||||||||||||
#' # Resolve tie when serious AE share a date by sorting with order argument | ||||||||||||
#' adsl <- tribble( | ||||||||||||
#' ~USUBJID, ~TRTSDT, ~EOSDT, | ||||||||||||
#' "01", ymd("2020-12-06"), ymd("2021-03-06"), | ||||||||||||
#' "02", ymd("2021-01-16"), ymd("2021-02-03") | ||||||||||||
#' ) %>% mutate(STUDYID = "AB42") | ||||||||||||
#' | ||||||||||||
#' ae <- tribble( | ||||||||||||
#' ~USUBJID, ~AESTDTC, ~AESEQ, ~AESER, ~AEDECOD, | ||||||||||||
#' "01", "2021-01-03", 1, "Y", "Flu", | ||||||||||||
#' "01", "2021-01-03", 2, "Y", "Cough", | ||||||||||||
#' "01", "2021-01-20", 3, "N", "Headache", | ||||||||||||
Comment on lines
+327
to
+336
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please align the columns in the |
||||||||||||
#' ) %>% mutate( | ||||||||||||
#' AESTDT = ymd(AESTDTC), | ||||||||||||
#' STUDYID = "AB42" | ||||||||||||
#' ) | ||||||||||||
#' | ||||||||||||
#' derive_param_tte( | ||||||||||||
#' dataset_adsl = adsl, | ||||||||||||
#' start_date = TRTSDT, | ||||||||||||
#' source_datasets = list(adsl = adsl, ae = ae), | ||||||||||||
#' event_conditions = list(event_source( | ||||||||||||
#' dataset_name = "ae", | ||||||||||||
#' date = AESTDT, | ||||||||||||
#' set_values_to = exprs( | ||||||||||||
#' EVENTDESC = "Serious AE", | ||||||||||||
#' SRCSEQ = AESEQ | ||||||||||||
#' ), | ||||||||||||
#' filter = AESER == "Y", | ||||||||||||
#' order = exprs(AESEQ) | ||||||||||||
#' )), | ||||||||||||
#' censor_conditions = list(censor_source( | ||||||||||||
#' dataset_name = "adsl", | ||||||||||||
#' date = EOSDT, | ||||||||||||
#' censor = 1, | ||||||||||||
#' set_values_to = exprs(EVENTDESC = "End of Study") | ||||||||||||
#' )), | ||||||||||||
#' set_values_to = exprs( | ||||||||||||
#' PARAMCD = "TTSAE", | ||||||||||||
#' PARAM = "Time to First Serious AE" | ||||||||||||
#' ) | ||||||||||||
#' ) | ||||||||||||
#' | ||||||||||||
derive_param_tte <- function(dataset = NULL, | ||||||||||||
dataset_adsl, | ||||||||||||
source_datasets, | ||||||||||||
|
@@ -322,8 +374,14 @@ derive_param_tte <- function(dataset = NULL, | |||||||||||
censor_conditions, | ||||||||||||
create_datetime = FALSE, | ||||||||||||
set_values_to, | ||||||||||||
subject_keys = get_admiral_option("subject_keys")) { | ||||||||||||
subject_keys = get_admiral_option("subject_keys"), | ||||||||||||
check_type = "warning") { | ||||||||||||
# checking and quoting # | ||||||||||||
check_type <- assert_character_scalar( | ||||||||||||
check_type, | ||||||||||||
values = c("warning", "message", "error", "none"), | ||||||||||||
case_sensitive = FALSE | ||||||||||||
) | ||||||||||||
assert_data_frame(dataset, optional = TRUE) | ||||||||||||
assert_vars(by_vars, optional = TRUE) | ||||||||||||
start_date <- assert_symbol(enexpr(start_date)) | ||||||||||||
|
@@ -373,16 +431,17 @@ derive_param_tte <- function(dataset = NULL, | |||||||||||
by_vars = by_vars | ||||||||||||
) | ||||||||||||
} | ||||||||||||
|
||||||||||||
tmp_event <- get_new_tmp_var(dataset) | ||||||||||||
|
||||||||||||
# determine events # | ||||||||||||
event_data <- filter_date_sources( | ||||||||||||
sources = event_conditions, | ||||||||||||
source_datasets = source_datasets, | ||||||||||||
by_vars = by_vars, | ||||||||||||
create_datetime = create_datetime, | ||||||||||||
subject_keys = subject_keys, | ||||||||||||
mode = "first" | ||||||||||||
mode = "first", | ||||||||||||
check_type = check_type | ||||||||||||
) %>% | ||||||||||||
mutate(!!tmp_event := 1L) | ||||||||||||
|
||||||||||||
|
@@ -393,7 +452,8 @@ derive_param_tte <- function(dataset = NULL, | |||||||||||
by_vars = by_vars, | ||||||||||||
create_datetime = create_datetime, | ||||||||||||
subject_keys = subject_keys, | ||||||||||||
mode = "last" | ||||||||||||
mode = "last", | ||||||||||||
check_type = check_type | ||||||||||||
) %>% | ||||||||||||
mutate(!!tmp_event := 0L) | ||||||||||||
|
||||||||||||
|
@@ -436,7 +496,8 @@ derive_param_tte <- function(dataset = NULL, | |||||||||||
bind_rows(event_data, censor_data), | ||||||||||||
by_vars = expr_c(subject_keys, by_vars), | ||||||||||||
order = exprs(!!tmp_event), | ||||||||||||
mode = "last" | ||||||||||||
mode = "last", | ||||||||||||
check_type = check_type | ||||||||||||
) %>% | ||||||||||||
inner_join( | ||||||||||||
adsl, | ||||||||||||
|
@@ -505,6 +566,16 @@ derive_param_tte <- function(dataset = NULL, | |||||||||||
#' | ||||||||||||
#' Permitted Values: `"first"`, `"last"` | ||||||||||||
#' | ||||||||||||
#' @param check_type Check uniqueness | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
(If the tag is indented, it is rendered as text.) |
||||||||||||
#' | ||||||||||||
#' If `"warning"`, `"message"`, or `"error"` is specified, the specified message is issued | ||||||||||||
#' if the observations of the input dataset are not unique with respect to the | ||||||||||||
#' by variables and the order. | ||||||||||||
Comment on lines
+572
to
+573
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||
#' | ||||||||||||
#' Default: `"none"` | ||||||||||||
#' | ||||||||||||
Comment on lines
+575
to
+576
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||
#' Permitted Values: `"none"`, `"warning"`, `"error"`, `"message"` | ||||||||||||
#' | ||||||||||||
ProfessorP-beep marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||
#' @details The following steps are performed to create the output dataset: | ||||||||||||
#' | ||||||||||||
#' \enumerate{ \item For each source dataset the observations as specified by | ||||||||||||
|
@@ -529,7 +600,7 @@ derive_param_tte <- function(dataset = NULL, | |||||||||||
#' @return A dataset with one observation per subject as described in the | ||||||||||||
#' "Details" section. | ||||||||||||
#' | ||||||||||||
#' @noRd | ||||||||||||
#' @keywords internal | ||||||||||||
ProfessorP-beep marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||
#' | ||||||||||||
#' @examples | ||||||||||||
#' library(tibble) | ||||||||||||
|
@@ -565,20 +636,22 @@ derive_param_tte <- function(dataset = NULL, | |||||||||||
#' ) | ||||||||||||
#' ) | ||||||||||||
#' | ||||||||||||
#' filter_date_sources( | ||||||||||||
#' admiral:::filter_date_sources( | ||||||||||||
#' sources = list(ttae), | ||||||||||||
#' source_datasets = list(adsl = adsl, ae = ae), | ||||||||||||
#' by_vars = exprs(AEDECOD), | ||||||||||||
#' create_datetime = FALSE, | ||||||||||||
#' subject_keys = get_admiral_option("subject_keys"), | ||||||||||||
#' mode = "first" | ||||||||||||
#' mode = "first", | ||||||||||||
#' check_type = "none" | ||||||||||||
#' ) | ||||||||||||
filter_date_sources <- function(sources, | ||||||||||||
source_datasets, | ||||||||||||
by_vars, | ||||||||||||
create_datetime = FALSE, | ||||||||||||
subject_keys, | ||||||||||||
mode) { | ||||||||||||
mode, | ||||||||||||
check_type = "none") { | ||||||||||||
ProfessorP-beep marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||
assert_list_of(sources, "tte_source") | ||||||||||||
assert_list_of(source_datasets, "data.frame") | ||||||||||||
assert_logical_scalar(create_datetime) | ||||||||||||
|
@@ -613,17 +686,49 @@ filter_date_sources <- function(sources, | |||||||||||
var = !!source_date_var, | ||||||||||||
dataset_name = sources[[i]]$dataset_name | ||||||||||||
) | ||||||||||||
data[[i]] <- source_dataset %>% | ||||||||||||
filter_if(sources[[i]]$filter) %>% | ||||||||||||
filter_extreme( | ||||||||||||
order = exprs(!!source_date_var), | ||||||||||||
by_vars = expr_c(subject_keys, by_vars), | ||||||||||||
mode = mode, | ||||||||||||
check_type = "none" | ||||||||||||
) | ||||||||||||
|
||||||||||||
# wrap filter_extreme in tryCatch to catch duplicate records and create a message | ||||||||||||
data[[i]] <- rlang::try_fetch( | ||||||||||||
{ | ||||||||||||
source_dataset %>% | ||||||||||||
filter_if(sources[[i]]$filter) %>% | ||||||||||||
filter_extreme( | ||||||||||||
order = expr_c(exprs(!!source_date_var), sources[[i]]$order), | ||||||||||||
by_vars = expr_c(subject_keys, by_vars), | ||||||||||||
mode = mode, | ||||||||||||
check_type = check_type | ||||||||||||
ProfessorP-beep marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||
) | ||||||||||||
}, | ||||||||||||
warning = function(cnd) { | ||||||||||||
# Handle warnings | ||||||||||||
if (grepl("duplicate records", conditionMessage(cnd))) { | ||||||||||||
cli::cli_warn(c( | ||||||||||||
"Dataset '{.val {sources[[i]]$dataset_name}}' contains duplicate records.", | ||||||||||||
"i Duplicates were identified based on variables: | ||||||||||||
{.val {paste(c(subject_keys, by_vars, source_date_var), collapse = ', ')}}." | ||||||||||||
)) | ||||||||||||
} | ||||||||||||
source_dataset %>% | ||||||||||||
filter_if(sources[[i]]$filter) %>% | ||||||||||||
arrange(!!!sources[[i]]$order) # Return filtered dataset even if a warning occurred | ||||||||||||
}, | ||||||||||||
error = function(err) { | ||||||||||||
cli::cli_abort(c( | ||||||||||||
"Duplicate records detected during processing.", | ||||||||||||
"x Duplicate records were found in dataset {.val {sources[[i]]$dataset_name}}.", | ||||||||||||
"i The duplicates were identified based on the following variables: | ||||||||||||
{.val {paste(c(subject_keys, by_vars, source_date_var), collapse = ', ')}}.", | ||||||||||||
"i Consider reviewing your `by_vars` or `order` argument to ensure uniqueness." | ||||||||||||
)) | ||||||||||||
}, | ||||||||||||
message = function(msg) { | ||||||||||||
cli::cli_inform(c( | ||||||||||||
"Processing dataset '{.val {sources[[i]]$dataset_name}}'...", | ||||||||||||
"i Filter and order criteria: {.val {paste(c(subject_keys, by_vars, | ||||||||||||
sources[[i]]$order), collapse = ', ')}}." | ||||||||||||
)) | ||||||||||||
} | ||||||||||||
Comment on lines
+701
to
+729
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Once #2592 is merged this could be simplified:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ProfessorP-beep , #2592 is merged now. |
||||||||||||
) | ||||||||||||
# add date variable and accompanying variables | ||||||||||||
|
||||||||||||
if (create_datetime) { | ||||||||||||
date_derv <- exprs(!!date_var := as_datetime(!!source_date_var)) | ||||||||||||
} else { | ||||||||||||
|
@@ -649,7 +754,7 @@ filter_date_sources <- function(sources, | |||||||||||
by_vars = expr_c(subject_keys, by_vars), | ||||||||||||
order = exprs(!!date_var), | ||||||||||||
mode = mode, | ||||||||||||
check_type = "none" | ||||||||||||
check_type = check_type | ||||||||||||
) | ||||||||||||
} | ||||||||||||
|
||||||||||||
|
@@ -782,6 +887,12 @@ extend_source_datasets <- function(source_datasets, | |||||||||||
#' SRCDOM = "ADSL", SRCVAR = "DTHDT")`. The values must be a symbol, a | ||||||||||||
#' character string, a numeric value, an expression, or `NA`. | ||||||||||||
#' | ||||||||||||
#' @param order Sort order | ||||||||||||
#' | ||||||||||||
#' An optional named list returned by `exprs()` defining additional variables | ||||||||||||
#' that the source dataset is sorted on after `date`. | ||||||||||||
#' | ||||||||||||
#' *Permitted Values:* list of variables created by `exprs()` e.g. `exprs(ASEQ)`. | ||||||||||||
#' | ||||||||||||
#' @keywords source_specifications | ||||||||||||
#' @family source_specifications | ||||||||||||
|
@@ -793,7 +904,8 @@ tte_source <- function(dataset_name, | |||||||||||
filter = NULL, | ||||||||||||
date, | ||||||||||||
censor = 0, | ||||||||||||
set_values_to = NULL) { | ||||||||||||
set_values_to = NULL, | ||||||||||||
order = order) { | ||||||||||||
out <- list( | ||||||||||||
dataset_name = assert_character_scalar(dataset_name), | ||||||||||||
filter = assert_filter_cond(enexpr(filter), optional = TRUE), | ||||||||||||
|
@@ -803,7 +915,8 @@ tte_source <- function(dataset_name, | |||||||||||
set_values_to, | ||||||||||||
named = TRUE, | ||||||||||||
optional = TRUE | ||||||||||||
) | ||||||||||||
), | ||||||||||||
order = order | ||||||||||||
) | ||||||||||||
class(out) <- c("tte_source", "source", "list") | ||||||||||||
out | ||||||||||||
|
@@ -844,13 +957,15 @@ tte_source <- function(dataset_name, | |||||||||||
event_source <- function(dataset_name, | ||||||||||||
filter = NULL, | ||||||||||||
date, | ||||||||||||
set_values_to = NULL) { | ||||||||||||
set_values_to = NULL, | ||||||||||||
order = NULL) { | ||||||||||||
out <- tte_source( | ||||||||||||
dataset_name = assert_character_scalar(dataset_name), | ||||||||||||
filter = !!enexpr(filter), | ||||||||||||
date = !!assert_expr(enexpr(date)), | ||||||||||||
censor = 0, | ||||||||||||
set_values_to = set_values_to | ||||||||||||
set_values_to = set_values_to, | ||||||||||||
order = order | ||||||||||||
) | ||||||||||||
class(out) <- c("event_source", class(out)) | ||||||||||||
out | ||||||||||||
|
@@ -891,13 +1006,15 @@ censor_source <- function(dataset_name, | |||||||||||
filter = NULL, | ||||||||||||
date, | ||||||||||||
censor = 1, | ||||||||||||
set_values_to = NULL) { | ||||||||||||
set_values_to = NULL, | ||||||||||||
order = NULL) { | ||||||||||||
out <- tte_source( | ||||||||||||
dataset_name = assert_character_scalar(dataset_name), | ||||||||||||
filter = !!enexpr(filter), | ||||||||||||
date = !!assert_expr(enexpr(date)), | ||||||||||||
censor = assert_integer_scalar(censor, subset = "positive"), | ||||||||||||
set_values_to = set_values_to | ||||||||||||
set_values_to = set_values_to, | ||||||||||||
order = order | ||||||||||||
) | ||||||||||||
class(out) <- c("censor_source", class(out)) | ||||||||||||
out | ||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -304,6 +304,7 @@ msec | |
nd | ||
occds | ||
onwards | ||
param | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is not necessary. You can check by calling There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. Will push soon. |
||
parttime | ||
pharmaverse | ||
pharmaverseadam | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.