Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

2481 bug the result of derive param tte depends on the sort order of the input #2569

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion R/derive_joined.R
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,7 @@ derive_vars_joined <- function(dataset,
derive_var_obs_number(
new_var = !!tmp_obs_nr,
by_vars = by_vars_left,
check_type = "none"
"none"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How come this argument got dropped?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure. Let me add it back in. I did a fresh pull before working on this again but I likely made a mistake

Copy link
Collaborator Author

@ProfessorP-beep ProfessorP-beep Nov 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its still there on my end. I'll push this again.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewing to see if I made any other mistakes.

)

data_joined <- get_joined_data(
Expand Down
38 changes: 29 additions & 9 deletions R/derive_param_tte.R
Original file line number Diff line number Diff line change
Expand Up @@ -322,8 +322,11 @@ derive_param_tte <- function(dataset = NULL,
censor_conditions,
create_datetime = FALSE,
set_values_to,
subject_keys = get_admiral_option("subject_keys")) {
# checking and quoting #
subject_keys = get_admiral_option("subject_keys"),
check_type = "warning") {
# Match check_type to valid admiral options
check_type <- rlang::arg_match(check_type, c("warning", "message", "error", "none"))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use the admiraldev assertions (assert_character_scalar()) and move it to the "checking and quoting" section below.

# checking and quoting #
assert_data_frame(dataset, optional = TRUE)
assert_vars(by_vars, optional = TRUE)
start_date <- assert_symbol(enexpr(start_date))
Expand Down Expand Up @@ -375,6 +378,7 @@ derive_param_tte <- function(dataset = NULL,
}

tmp_event <- get_new_tmp_var(dataset)

# determine events #
event_data <- filter_date_sources(
sources = event_conditions,
Expand All @@ -386,6 +390,11 @@ derive_param_tte <- function(dataset = NULL,
) %>%
mutate(!!tmp_event := 1L)

#check for duplicates in event_data
signal_duplicate_records(dataset = event_data,
by_vars = expr_c(by_vars, subject_keys),
cnd_type = check_type)

# determine censoring observations #
censor_data <- filter_date_sources(
sources = censor_conditions,
Expand All @@ -397,6 +406,11 @@ derive_param_tte <- function(dataset = NULL,
) %>%
mutate(!!tmp_event := 0L)

#check for duplicates in censor_data
signal_duplicate_records(dataset = censor_data,
by_vars = expr_c(by_vars, subject_keys),
cnd_type = check_type)

# determine variable to add from ADSL #
if (create_datetime) {
date_var <- sym("ADTM")
Expand Down Expand Up @@ -463,7 +477,7 @@ derive_param_tte <- function(dataset = NULL,
}
}

# add new parameter to input dataset #
# add new parameter to input dataset #
bind_rows(dataset, new_param)
}

Expand Down Expand Up @@ -793,7 +807,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),
Expand All @@ -803,7 +818,8 @@ tte_source <- function(dataset_name,
set_values_to,
named = TRUE,
optional = TRUE
)
),
order = order
)
class(out) <- c("tte_source", "source", "list")
out
Expand Down Expand Up @@ -844,13 +860,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
Expand Down Expand Up @@ -891,13 +909,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
Expand Down
3 changes: 2 additions & 1 deletion man/censor_source.Rd

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

3 changes: 2 additions & 1 deletion man/derive_param_tte.Rd

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

8 changes: 7 additions & 1 deletion man/event_source.Rd

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

9 changes: 8 additions & 1 deletion man/tte_source.Rd

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

15 changes: 0 additions & 15 deletions tests/testthat/_snaps/derive_param_tte.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,21 +34,6 @@
! For some values of "PARAMCD" there is more than one value of "AEDECOD"
i Call `admiral::get_one_to_many_dataset()` to get all one-to-many values.

# derive_param_tte Test 9: errors if set_values_to contains invalid expressions

Code
derive_param_tte(dataset_adsl = adsl, by_vars = exprs(AEDECOD), start_date = TRTSDT,
event_conditions = list(ttae), censor_conditions = list(eos), source_datasets = list(
adsl = adsl, ae = ae), set_values_to = exprs(PARAMCD = paste0("TTAE",
as.numeric(as.factor(AEDECOD))), PARAM = past("Time to First", AEDECOD,
"Adverse Event"), PARCAT1 = "TTAE", PARCAT2 = AEDECOD))
Condition
Error in `process_set_values_to()`:
! Assigning variables failed!
* `set_values_to = exprs(PARAMCD = paste0("TTAE", as.numeric(as.factor(AEDECOD))), PARAM = past("Time to First", AEDECOD, "Adverse Event"), PARCAT1 = TTAE, PARCAT2 = AEDECOD)`
See error message below:
i In argument: `PARAM = past("Time to First", AEDECOD, "Adverse Event")`. Caused by error in `past()`: ! could not find function "past"

# list_tte_source_objects Test 13: error is issued if package does not exist

Code
Expand Down
Loading