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.

Sign up for GitHub

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 #2068 Updated traceability_vars to set_values_to #2079

Merged
merged 4 commits into from
Sep 1, 2023
Merged

Closes #2068 Updated traceability_vars to set_values_to #2079

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ae842d2
Closes #2068 Updated `traceability_vars` to `set_values_to`
sadchla-codes Aug 29, 2023
0077aa9
Merge branch 'devel' into 2068_replace_traceability_vars@devel
sadchla-codes Aug 29, 2023
5c40bcf
Merge branch 'devel' into 2068_replace_traceability_vars@devel
sadchla-codes Aug 30, 2023
166de8d
Clarify the explanation for the traceability_vars replacement
sadchla-codes Aug 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@

- Templates, vignettes, and other uses of `{admiral.test}` SDTM data are updated to use `{pharmaversesdtm}` instead. (#2040)

- The `traceability_vars` argument in `date_source()` and `dthcaus_source` were deprecated in favor of `set_values_to`. The `date_source()` function creates a date_source object as input for derive_var_extreme_dt() and derive_var_extreme_dtm(),users now have define the traceability variables by assigning those variables to the `set_values_to`argument.Similarly, the `dthcaus_source` creates a dthcaus_source Object. (#2068)


## Breaking Changes
- The `compute_duration(type)` argument added the `"duration"` type calculation, and this is the new default (previously `"interval"` differences were returned). See function help file for details on the difference between `"duration"` and `"interval"` calculations. (#1875)
Expand Down
30 changes: 23 additions & 7 deletions R/derive_var_dthcaus.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@
#' date = convert_dtc_to_dt(AEDTHDTC),
#' mode = "first",
#' dthcaus = AEDECOD,
#' traceability_vars = exprs(DTHDOM = "AE", DTHSEQ = AESEQ)
#' set_values_to = exprs(DTHDOM = "AE", DTHSEQ = AESEQ)
#' )
#'
#' src_ds <- dthcaus_source(
Expand All @@ -90,7 +90,7 @@
#' date = convert_dtc_to_dt(DSSTDTC),
#' mode = "first",
#' dthcaus = DSTERM,
#' traceability_vars = exprs(DTHDOM = "DS", DTHSEQ = DSSEQ)
#' set_values_to = exprs(DTHDOM = "DS", DTHSEQ = DSSEQ)
#' )
#'
#' derive_var_dthcaus(adsl, src_ae, src_ds, source_datasets = list(ae = ae, ds = ds))
Expand All @@ -102,7 +102,7 @@
#' date = convert_dtc_to_dt(AEDTHDTC),
#' mode = "first",
#' dthcaus = AEDECOD,
#' traceability_vars = exprs(DTHDOM = "AE", DTHSEQ = AESEQ)
#' set_values_to = exprs(DTHDOM = "AE", DTHSEQ = AESEQ)
#' )
#'
#' ds <- mutate(
Expand All @@ -116,7 +116,7 @@
#' date = DSSTDT,
#' mode = "first",
#' dthcaus = DSTERM,
#' traceability_vars = exprs(DTHDOM = "DS", DTHSEQ = DSSEQ)
#' set_values_to = exprs(DTHDOM = "DS", DTHSEQ = DSSEQ)
#' )
#'
#' src_ds_post <- dthcaus_source(
Expand All @@ -125,7 +125,7 @@
#' date = DSSTDT,
#' mode = "first",
#' dthcaus = "POST STUDY: UNKNOWN CAUSE",
#' traceability_vars = exprs(DTHDOM = "DS", DTHSEQ = DSSEQ)
#' set_values_to = exprs(DTHDOM = "DS", DTHSEQ = DSSEQ)
#' )
#'
#' derive_var_dthcaus(
Expand Down Expand Up @@ -267,6 +267,7 @@ derive_var_dthcaus <- function(dataset,
#' the results is assigned to `DTHCAUS`; if a string literal, e.g. `"Adverse
#' Event"`, it is the fixed value to be assigned to `DTHCAUS`.
#'
#'
#' @param traceability_vars A named list returned by [`exprs()`] listing the
#' traceability variables, e.g. `exprs(DTHDOM = "DS", DTHSEQ = DSSEQ)`. The
#' left-hand side (names of the list elements) gives the names of the
Expand All @@ -275,6 +276,10 @@ derive_var_dthcaus <- function(dataset,
#' returned dataset. These can be either strings, numbers, symbols, or
#' expressions referring to existing variables.
#'
#' `r lifecycle::badge("deprecated")` Please use `set_values_to` instead.
#'
#' @param set_values_to Variables to be set to trace the source dataset
#'
#' @keywords source_specifications
#' @family source_specifications
#'
Expand Down Expand Up @@ -309,15 +314,26 @@ dthcaus_source <- function(dataset_name,
order = NULL,
mode = "first",
dthcaus,
traceability_vars = NULL) {
set_values_to = NULL,
traceability_vars = NULL
) {
if (!is.null(traceability_vars)) {
deprecate_warn(
"0.12.0",
"dthcaus_source(traceability_vars = )",
"dthcaus_source(set_values_to = )"
)
set_values_to <- traceability_vars
}

out <- list(
dataset_name = assert_character_scalar(dataset_name),
filter = assert_filter_cond(enexpr(filter), optional = TRUE),
date = assert_expr(enexpr(date)),
order = assert_expr_list(order, optional = TRUE),
mode = assert_character_scalar(mode, values = c("first", "last"), case_sensitive = FALSE),
dthcaus = assert_expr(enexpr(dthcaus)),
traceability = assert_expr_list(traceability_vars, named = TRUE, optional = TRUE)
traceability = assert_expr_list(set_values_to, named = TRUE, optional = TRUE)
)
class(out) <- c("dthcaus_source", "source", "list")
out
Expand Down
52 changes: 33 additions & 19 deletions R/derive_var_extreme_date.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
#' `date` element. If this is a date variable (rather than datetime), then the
#' time is imputed as `"00:00:00"`.
#'
#' 1. The variables specified by the `traceability_vars` element are added.
#' 1. The variables specified by the `set_values_to` element are added.
#'
#' 1. The selected observations of all source datasets are combined into a
#' single dataset.
Expand Down Expand Up @@ -177,7 +177,7 @@
#' ae_start <- date_source(
#' dataset_name = "ae",
#' date = convert_dtc_to_dtm(AESTDTC, highest_imputation = "M"),
#' traceability_vars = exprs(
#' set_values_to = exprs(
#' LALVDOM = "AE",
#' LALVSEQ = AESEQ,
#' LALVVAR = "AESTDTC"
Expand All @@ -187,7 +187,7 @@
#' ae_end <- date_source(
#' dataset_name = "ae",
#' date = convert_dtc_to_dtm(AEENDTC, highest_imputation = "M"),
#' traceability_vars = exprs(
#' set_values_to = exprs(
#' LALVDOM = "AE",
#' LALVSEQ = AESEQ,
#' LALVVAR = "AEENDTC"
Expand All @@ -196,7 +196,7 @@
#' lb_date <- date_source(
#' dataset_name = "lb",
#' date = convert_dtc_to_dtm(LBDTC),
#' traceability_vars = exprs(
#' set_values_to = exprs(
#' LALVDOM = "LB",
#' LALVSEQ = LBSEQ,
#' LALVVAR = "LBDTC"
Expand All @@ -206,7 +206,7 @@
#' adsl_date <- date_source(
#' dataset_name = "adsl",
#' date = TRTEDTM,
#' traceability_vars = exprs(
#' set_values_to = exprs(
#' LALVDOM = "ADSL",
#' LALVSEQ = NA_integer_,
#' LALVVAR = "TRTEDTM"
Expand Down Expand Up @@ -263,8 +263,8 @@ derive_var_extreme_dtm <- function(dataset,
for (i in seq_along(sources)) {
if (i > 1) {
warn_if_inconsistent_list(
base = sources[[i - 1]]$traceability_vars,
compare = sources[[i]]$traceability_vars,
base = sources[[i - 1]]$set_values_to,
compare = sources[[i]]$set_values_to,
list_name = "date_source()",
i = i
)
Expand All @@ -289,11 +289,11 @@ derive_var_extreme_dtm <- function(dataset,
dataset_name = source_dataset_name
)

if (!is.null(sources[[i]]$traceability_vars)) {
warn_if_vars_exist(source_dataset, names(sources[[i]]$traceability_vars))
if (!is.null(sources[[i]]$set_values_to)) {
warn_if_vars_exist(source_dataset, names(sources[[i]]$set_values_to))
assert_data_frame(
source_dataset,
required_vars = get_source_vars(sources[[i]]$traceability_vars)
required_vars = get_source_vars(sources[[i]]$set_values_to)
)
}

Expand All @@ -310,7 +310,7 @@ derive_var_extreme_dtm <- function(dataset,
add_data[[i]] <- transmute(
Copy link
Collaborator

Choose a reason for hiding this comment

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

@pharmaverse/admiral Wondering if we should rewrite the code-section as well to keep up with our current programming practices but otherwise I think it should be fine?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hey @bms63 @manciniedoardo @bundfussr Are you able to provide some feedback on @zdz2101 question above, or should I merge this PR for now and the question above could be address in a separate issue?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@zdz2101 What do you mean by code-section? I'm not following :(

Copy link
Collaborator

Choose a reason for hiding this comment

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

@bms63 little two-parter, semi-related to function re-work type of stuff:

  1. usually with the set_values_to argument related stuff, we use process_set_values_to() from admiraldev
  2. dplyr::transmute() was superseded earlier this year, we don't use it too often in our codebase anyhow but if we want to harmonize our code-base such that all functions are semi-consistently this is kinda part of it

Personal opinion, I'm okay with just merging this in, just thought I'd throw it out there. It'd probably be a back of the backlog type of task, kinda that mantra of don't fix wha'ts not broken type of thing

Copy link
Collaborator

Choose a reason for hiding this comment

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

My view is that harmonisation of the code base is important; within the harmonisation efforts priority should be given to user-facing harmonisation so that we can move faster towards admiral 1.0.0. Thus I don't mind if a change like this is relegated to the backlog and we get to it when we get to it.

add_data[[i]],
!!!subject_keys,
!!!sources[[i]]$traceability_vars,
!!!sources[[i]]$set_values_to,
!!new_var := convert_date_to_dtm(!!date_var)
)
}
Expand Down Expand Up @@ -350,7 +350,7 @@ derive_var_extreme_dtm <- function(dataset,
#' 1. The new variable is set to the variable or expression specified by the
#' `date` element.
#'
#' 1. The variables specified by the `traceability_vars` element are added.
#' 1. The variables specified by the `set_values_to` element are added.
#'
#' 1. The selected observations of all source datasets are combined into a
#' single dataset.
Expand Down Expand Up @@ -489,7 +489,7 @@ derive_var_extreme_dtm <- function(dataset,
#' ae_start <- date_source(
#' dataset_name = "ae",
#' date = convert_dtc_to_dt(AESTDTC, highest_imputation = "M"),
#' traceability_vars = exprs(
#' set_values_to = exprs(
#' LALVDOM = "AE",
#' LALVSEQ = AESEQ,
#' LALVVAR = "AESTDTC"
Expand All @@ -499,7 +499,7 @@ derive_var_extreme_dtm <- function(dataset,
#' ae_end <- date_source(
#' dataset_name = "ae",
#' date = convert_dtc_to_dt(AEENDTC, highest_imputation = "M"),
#' traceability_vars = exprs(
#' set_values_to = exprs(
#' LALVDOM = "AE",
#' LALVSEQ = AESEQ,
#' LALVVAR = "AEENDTC"
Expand All @@ -509,7 +509,7 @@ derive_var_extreme_dtm <- function(dataset,
#' lb_date <- date_source(
#' dataset_name = "lb",
#' date = convert_dtc_to_dt(LBDTC),
#' traceability_vars = exprs(
#' set_values_to = exprs(
#' LALVDOM = "LB",
#' LALVSEQ = LBSEQ,
#' LALVVAR = "LBDTC"
Expand All @@ -519,7 +519,7 @@ derive_var_extreme_dtm <- function(dataset,
#' adsl_date <- date_source(
#' dataset_name = "adsl",
#' date = TRTEDT,
#' traceability_vars = exprs(
#' set_values_to = exprs(
#' LALVDOM = "ADSL",
#' LALVSEQ = NA_integer_,
#' LALVVAR = "TRTEDT"
Expand Down Expand Up @@ -578,6 +578,9 @@ derive_var_extreme_dt <- function(dataset,
#' = "AESTDTC")`. The values must be a symbol, a character string, a numeric,
#' an expression, or `NA`.
#'
#' `r lifecycle::badge("deprecated")` Please use `set_values_to` instead.
#'
#' @param set_values_to Variables to be set
#'
#' @seealso [derive_var_extreme_dtm()], [derive_var_extreme_dt()]
#'
Expand Down Expand Up @@ -607,20 +610,31 @@ derive_var_extreme_dt <- function(dataset,
#' death_date <- date_source(
#' dataset_name = "adsl",
#' date = DTHDT,
#' traceability_vars = exprs(
#' set_values_to = exprs(
#' LALVDOM = "ADSL",
#' LALVVAR = "DTHDT"
#' )
#' )
date_source <- function(dataset_name,
filter = NULL,
date,
traceability_vars = NULL) {
traceability_vars = NULL,
set_values_to = NULL) {
if (!is.null(traceability_vars)) {
deprecate_warn(
"0.12.0",
"date_source(traceability_vars = )",
"date_source(set_values_to = )"
)
set_values_to <- traceability_vars

}

out <- list(
dataset_name = assert_character_scalar(dataset_name),
filter = assert_filter_cond(enexpr(filter), optional = TRUE),
date = assert_expr(enexpr(date)),
traceability_vars = assert_expr_list(traceability_vars, named = TRUE, optional = TRUE)
set_values_to = assert_expr_list(set_values_to, named = TRUE, optional = TRUE)
)
class(out) <- c("date_source", "source", "list")
out
Expand Down
16 changes: 13 additions & 3 deletions man/date_source.Rd
View file
Open in desktop

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

10 changes: 5 additions & 5 deletions man/derive_var_dthcaus.Rd
View file
Open in desktop

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

10 changes: 5 additions & 5 deletions man/derive_var_extreme_dt.Rd
View file
Open in desktop

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

Loading