From 5146352842a05db64ef01c1432484c440b88a14c Mon Sep 17 00:00:00 2001 From: Aleksander Chlebowski Date: Thu, 6 Feb 2025 21:13:48 +0100 Subject: [PATCH 01/17] add delayed_datasets --- R/delayed_datasets.R | 101 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 R/delayed_datasets.R diff --git a/R/delayed_datasets.R b/R/delayed_datasets.R new file mode 100644 index 00000000..44f36efa --- /dev/null +++ b/R/delayed_datasets.R @@ -0,0 +1,101 @@ +#' Delayed datasets +#' +#' Generate `delayed_data_extract_spec` without prior knowledge of the data. +#' +#' documentation WIP +#' +#' `delayed_datasets` is a character string with class `delayed_datasets` +#' and attribute `datasets` which is set to `x`. +#' The attribute specifies a wishlist of datasets for which `delayed_des` are to be created. +#' +#' `delayed_data_extract_specs` are resolved as follows: +#' - `data_extract_specs` are returned as is +#' - `delayed_data_extract_specs` where `dataname` is `character` are returned as is +#' - `delayed_data_extract_specs` where `dataname` is `delayed_datasets` is first confronted +#' with names of datasets in the app and has its `datasets` attribute updated, +#' and then is converted to a list of `delayed_data_extract_spec`s of the same length as +#' the updated `datasets` attribute. +#' +#' @param x (`character`) set of dataset names for wchich `delayed_data_extract_spec`s will be created; +#' set to `"all"` to use all available datasets +#' @param des (`data_extract_spec` or `list` thereof) see `Details` +#' @param datasets (`character`) vector of dataset for which to resolve + +#' @name delayed_datasets +NULL + +#' @rdname delayed_datasets +#' @export +delayed_datasets <- function(x = "all") { + structure( + "delayed_datasets", + class = c("delayed_datasets", "delayed_data", "character"), + datasets = x + ) +} + +#' @rdname delayed_datasets +#' @export +resolve_delayed_datasets <- function(des, datasets) { + .resolve_delayed_datasets(.update_delayed_datasets(des, datasets)) +} + +#' @keywords internal +#' @noRd +.update_delayed_datasets <- function(des, datasets) { + .horse <- function(des, datasets) { + delayed <- attr(des, "datasets", exact = TRUE) + delayed <- + if (identical(delayed, "all")) { + datasets + } else { + intersect(delayed, datasets) + } + attr(des, "datasets") <- delayed + des + } + + rapply(des, .horse, "delayed_datasets", how = "replace", datasets = datasets) +} + +#' @keywords internal +#' @noRd +.resolve_delayed_datasets <- function(des) { + .horse <- function(des) { + if (!inherits(des$dataname, "delayed_datasets")) return(des) + lapply(attr(des$dataname, "datasets", exact = TRUE), function(dataset) { + rapply(des, f = function(...) dataset, "delayed_datasets", how = "replace") + }) + } + + if (inherits(des, "delayed_data_extract_spec")) return(.horse(des)) + lapply(des, .resolve_delayed_datasets) +} + +#' ensure that all delayed_datasets in a delayed_des are the same +assert_delayed_datesets_identical <- function(x) { + checkmate::assert_class(x, "data_extract_spec") + if (inherits(x, "delayed_data_extract_spec")) { + master <- x$dataname + if (inherits(master, "delayed_datasets")) { + error_msg <- paste0(deparse1(match.call()), ": delayed_datasets identity violated") + slaves <- rapply(x, function(xx) xx, "delayed_datasets", how = "unlist") + slaves_datasets <- rapply(x, function(xx) attr(xx, "datasets"), "delayed_datasets", how = "unlist") + Reduce( + function(x1, x2) { + if (identical(x1, x2)) x2 else stop(error_msg, call. = FALSE) + }, + slaves, + init = as.vector(master) + ) + Reduce( + function(x1, x2) { + if (identical(x1, x2)) x2 else stop(error_msg, call. = FALSE) + }, + slaves_datasets, + init = attr(master, "datasets") + ) + } + } + x +} From c7ca7c286c14ac6b27e2e400a7bc57f6bcc938bd Mon Sep 17 00:00:00 2001 From: Aleksander Chlebowski Date: Thu, 6 Feb 2025 21:16:05 +0100 Subject: [PATCH 02/17] recognize delayed_datasets in get_extract_datanames --- R/data_extract_datanames.R | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/R/data_extract_datanames.R b/R/data_extract_datanames.R index 7437cb3b..f52f4490 100644 --- a/R/data_extract_datanames.R +++ b/R/data_extract_datanames.R @@ -61,7 +61,20 @@ get_extract_datanames <- function(data_extracts) { } }) - unique(unlist(datanames)) + .extract_delayed_datasets <- function(x) { + if (inherits(x, "delayed_datasets")) { + attr(x, "datasets", exact = TRUE) + } else { + x + } + } + datanames <- rapply(datanames, .extract_delayed_datasets) + + if (any(datanames == "all")) { + "all" + } else { + unique(datanames) + } } #' Verify uniform dataset source across data extract specification @@ -82,5 +95,5 @@ get_extract_datanames <- function(data_extracts) { is_single_dataset <- function(...) { data_extract_spec <- list(...) dataset_names <- get_extract_datanames(data_extract_spec) - length(dataset_names) == 1 + length(dataset_names) == 1L && dataset_names != "all" } From 9ae2204983476e1d38aa02521084c8c3b20c6722 Mon Sep 17 00:00:00 2001 From: Aleksander Chlebowski Date: Thu, 6 Feb 2025 21:17:05 +0100 Subject: [PATCH 03/17] resolve delayed_datasets in data_extract_multiple_srv --- R/data_extract_module.R | 1 + 1 file changed, 1 insertion(+) diff --git a/R/data_extract_module.R b/R/data_extract_module.R index f541de50..baa6bce6 100644 --- a/R/data_extract_module.R +++ b/R/data_extract_module.R @@ -764,6 +764,7 @@ data_extract_multiple_srv.list <- function(data_extract, ) data_extract <- Filter(Negate(is.null), data_extract) + data_extract <- resolve_delayed_datasets(data_extract, names(datasets)) if (is.function(select_validation_rule)) { select_validation_rule <- sapply( From 389dce66b0449e18de5cb063ce1c28ae912d90d7 Mon Sep 17 00:00:00 2001 From: Aleksander Chlebowski Date: Sat, 8 Feb 2025 00:03:08 +0100 Subject: [PATCH 04/17] add .integrate stage in resolve_delayed_datasets --- R/delayed_datasets.R | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/R/delayed_datasets.R b/R/delayed_datasets.R index 44f36efa..ba72bf8c 100644 --- a/R/delayed_datasets.R +++ b/R/delayed_datasets.R @@ -37,7 +37,16 @@ delayed_datasets <- function(x = "all") { #' @rdname delayed_datasets #' @export resolve_delayed_datasets <- function(des, datasets) { - .resolve_delayed_datasets(.update_delayed_datasets(des, datasets)) + .integrate <- function(x) { + if (inherits(x, "delayed_data_extract_spec")) return(x) + if (checkmate::test_list(x, "list", len = 1L) && + checkmate::test_list(x[[1L]], "delayed_data_extract_spec")) { + return(x[[1L]]) + } + lapply(x, .integrate) + } + + .resolve_delayed_datasets(.update_delayed_datasets(des, datasets)) |> .integrate() } #' @keywords internal From b2b7e31fc2258deae9f8bbca0b2ec43e89b4e2f7 Mon Sep 17 00:00:00 2001 From: Aleksander Chlebowski Date: Mon, 10 Feb 2025 12:05:20 +0100 Subject: [PATCH 05/17] add unit tests for delayed_datasets --- tests/testthat/test-delayed_datasets.R | 91 ++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 tests/testthat/test-delayed_datasets.R diff --git a/tests/testthat/test-delayed_datasets.R b/tests/testthat/test-delayed_datasets.R new file mode 100644 index 00000000..c6450708 --- /dev/null +++ b/tests/testthat/test-delayed_datasets.R @@ -0,0 +1,91 @@ +data <- teal.data::cdisc_data( + ADSL = teal.data::rADSL, + ADAE = teal.data::rADAE +) + +des_current1 <- data_extract_spec( + dataname = "ADSL", + select = select_spec( + choices = variable_choices( + data = "ADSL", + subset = function(data) names(Filter(is.numeric, data)) + ), + selected = first_choice() + ) +) + +des_current2 <- data_extract_spec( + dataname = "ADAE", + select = select_spec( + choices = variable_choices( + data = "ADAE", + subset = function(data) names(Filter(is.numeric, data)) + ), + selected = first_choice() + ) +) + +des_delayed <- data_extract_spec( + dataname = delayed_datasets(), + select = select_spec( + choices = variable_choices( + data = delayed_datasets(), + subset = function(data) names(Filter(is.numeric, data)) + ), + selected = first_choice() + ) +) + +des_resolved <- list( + des_current1, + des_current2 +) + + + +testthat::test_that("single current ddes is unchanged", { + testthat::expect_identical( + des_current1 |> resolve_delayed_datasets(names(data)), + des_current1 + ) +}) + +testthat::test_that("single delayed ddes is resolved into list of length(names(data))", { + testthat::expect_equal( + des_delayed |> resolve_delayed_datasets(names(data)), + des_resolved, + check.environment = FALSE + ) +}) + +testthat::test_that("resolved des replaces parent level in nested list of length 1", { + # this reproduces what happens in data_extract_multiple_srv.list + testthat::expect_equal( + list(des_current1, des_delayed) |> resolve_delayed_datasets(names(data)), + list(des_current1, des_resolved), + check.environment = FALSE + ) + testthat::expect_equal( + list(list(des_current1), list(des_delayed)) |> resolve_delayed_datasets(names(data)), + list(list(des_current1), des_resolved), + check.environment = FALSE + ) +}) + +testthat::test_that("ddes with specified datasets resolves to intersection of those and the available ones", { + des_delayed_subset <- data_extract_spec( + dataname = delayed_datasets(c("ADSL", "ADEX")), + select = select_spec( + choices = variable_choices( + data = delayed_datasets(c("ADSL", "ADEX")), + subset = function(data) names(Filter(is.numeric, data)) + ), + selected = first_choice() + ) + ) + testthat::expect_equal( + des_delayed_subset |> resolve_delayed_datasets(names(data)), + list(des_current1), + check.environment = FALSE + ) +}) From 6616eab87657e54d189b0e772ae97566dc008773 Mon Sep 17 00:00:00 2001 From: Aleksander Chlebowski Date: Wed, 12 Feb 2025 14:41:54 +0100 Subject: [PATCH 06/17] rewrite data_extract_module to create entire UI server side --- R/data_extract_module.R | 157 +++++++++++++++++++++++----------------- 1 file changed, 90 insertions(+), 67 deletions(-) diff --git a/R/data_extract_module.R b/R/data_extract_module.R index baa6bce6..8919fbaa 100644 --- a/R/data_extract_module.R +++ b/R/data_extract_module.R @@ -124,74 +124,14 @@ cond_data_extract_single_ui <- function(ns, single_data_extract_spec) { data_extract_ui <- function(id, label, data_extract_spec, is_single_dataset = FALSE) { ns <- NS(id) - if (inherits(data_extract_spec, "data_extract_spec")) { - data_extract_spec <- list(data_extract_spec) - } - check_data_extract_spec(data_extract_spec) - - if (is.null(data_extract_spec)) { - return(helpText(sprintf("Data extraction with label '%s' is NULL. Please contact the app author.", label))) - } - stopifnot( - `more than one dataset in data_extract_spec but is_single_dataset parameter is set to TRUE` = - !is_single_dataset || length(data_extract_spec) == 1 - ) - - dataset_names <- vapply( - data_extract_spec, - function(x) x$dataname, - character(1), - USE.NAMES = FALSE - ) - - stopifnot(`list contains data_extract_spec objects with the same dataset` = all(!duplicated(dataset_names))) - - dataset_input <- if (is_single_dataset) { - NULL - } else { - if (length(dataset_names) == 1) { - if ((is.null(data_extract_spec[[1]]$filter)) && - ( - !is.null(data_extract_spec[[1]]$select$fixed) && - data_extract_spec[[1]]$select$fixed == TRUE - )) { - NULL - } else { - helpText("Dataset:", tags$code(dataset_names)) - } - } else { - teal.widgets::optionalSelectInput( - inputId = ns("dataset"), - label = "Dataset", - choices = dataset_names, - selected = dataset_names[1], - multiple = FALSE - ) - } - } tagList( - include_css_files(pattern = "data_extract"), - tags$div( - class = "data-extract", - tags$label(label), - dataset_input, - if (length(dataset_names) == 1) { - data_extract_single_ui( - id = ns(id_for_dataset(dataset_names)), - single_data_extract_spec = data_extract_spec[[1]] - ) - } else { - do.call( - div, - unname(lapply( - data_extract_spec, - function(x) { - cond_data_extract_single_ui(ns, x) - } - )) - ) - } - ) + # Pass arguments to server function. + div( + checkboxInput(ns("is_single_dataset"), label = NULL, value = is_single_dataset), + textInput(ns("data_extract_label"), label = NULL, value = label), + style = "display: none;" + ), + uiOutput(ns("data_extract_ui_container")) ) } @@ -562,6 +502,89 @@ data_extract_srv.list <- function(id, ) } }) + + + output$data_extract_ui_container <- renderUI({ + ns <- session$ns + + logger::log_debug( + "initializing data_extract_ui with datasets: { paste(names(datasets), collapse = ', ') }." + ) + + if (inherits(data_extract_spec, "data_extract_spec")) { + data_extract_spec <- list(data_extract_spec) + } + check_data_extract_spec(data_extract_spec) + + if (is.null(data_extract_spec)) { + return(helpText(sprintf("Data extraction with label '%s' is NULL. Please contact the app author.", label))) + } + stopifnot( + `more than one dataset in data_extract_spec but is_single_dataset parameter is set to TRUE` = + isFALSE(input$is_single_dataset) || length(data_extract_spec) == 1 + ) + + dataset_names <- vapply( + data_extract_spec, + function(x) x$dataname, + character(1), + USE.NAMES = FALSE + ) + + if (anyDuplicated(dataset_names) != 0L) { + stop("list contains data_extract_spec objects with the same dataset") + } + + dataset_input <- + if (isTRUE(input$is_single_dataset)) { + # if (FALSE) { + NULL + } else { + if (length(dataset_names) == 1) { + if ((is.null(data_extract_spec[[1]]$filter)) && + ( + !is.null(data_extract_spec[[1]]$select$fixed) && + data_extract_spec[[1]]$select$fixed == TRUE + )) { + NULL + } else { + helpText("Dataset:", tags$code(dataset_names)) + } + } else { + teal.widgets::optionalSelectInput( + inputId = ns("dataset"), + label = "Dataset", + choices = dataset_names, + selected = dataset_names[1], + multiple = FALSE + ) + } + } + tagList( + include_css_files(pattern = "data_extract"), + tags$div( + class = "data-extract", + tags$label(input$data_extract_label), + dataset_input, + if (length(dataset_names) == 1) { + data_extract_single_ui( + id = ns(id_for_dataset(dataset_names)), + single_data_extract_spec = data_extract_spec[[1]] + ) + } else { + do.call( + div, + unname(lapply( + data_extract_spec, + function(x) { + cond_data_extract_single_ui(ns, x) + } + )) + ) + } + ) + ) + }) filter_and_select_reactive } ) From 9f2622a539a54b0b30d4a7eb8fd25ebce05b4e3a Mon Sep 17 00:00:00 2001 From: Aleksander Chlebowski Date: Wed, 12 Feb 2025 15:07:59 +0100 Subject: [PATCH 07/17] style and comment code --- R/delayed_datasets.R | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/R/delayed_datasets.R b/R/delayed_datasets.R index ba72bf8c..fb5e89fc 100644 --- a/R/delayed_datasets.R +++ b/R/delayed_datasets.R @@ -37,6 +37,9 @@ delayed_datasets <- function(x = "all") { #' @rdname delayed_datasets #' @export resolve_delayed_datasets <- function(des, datasets) { + # When used on a ddes with delayed_dataset that is in a list + # .unfold_delayed_datasets creates a list(list(ddes, ddes)) structure + # where list(ddes, ddes) is expected. One list level has to be collapsed. .integrate <- function(x) { if (inherits(x, "delayed_data_extract_spec")) return(x) if (checkmate::test_list(x, "list", len = 1L) && @@ -46,12 +49,14 @@ resolve_delayed_datasets <- function(des, datasets) { lapply(x, .integrate) } - .resolve_delayed_datasets(.update_delayed_datasets(des, datasets)) |> .integrate() + .unfold_delayed_datasets(des, datasets) |> + .resolve_delayed_datasets() |> + .integrate() } #' @keywords internal #' @noRd -.update_delayed_datasets <- function(des, datasets) { +.unfold_delayed_datasets <- function(des, datasets) { .horse <- function(des, datasets) { delayed <- attr(des, "datasets", exact = TRUE) delayed <- From e93f644de7148875818723688300c3ae7fc80812 Mon Sep 17 00:00:00 2001 From: Aleksander Chlebowski Date: Wed, 12 Feb 2025 16:27:46 +0100 Subject: [PATCH 08/17] check data_extract_specs upon creation --- R/data_extract_spec.R | 4 +- R/delayed_datasets.R | 135 ++++++++++------------------------- R/resolve_delayed_datasets.R | 124 ++++++++++++++++++++++++++++++++ 3 files changed, 164 insertions(+), 99 deletions(-) create mode 100644 R/resolve_delayed_datasets.R diff --git a/R/data_extract_spec.R b/R/data_extract_spec.R index 517f2861..71b8f5fe 100644 --- a/R/data_extract_spec.R +++ b/R/data_extract_spec.R @@ -111,7 +111,7 @@ data_extract_spec <- function(dataname, select = NULL, filter = NULL, reshape = for (idx in seq_along(filter)) filter[[idx]]$dataname <- dataname - if ( + ans <- if ( inherits(select, "delayed_select_spec") || any(vapply(filter, inherits, logical(1), "delayed_filter_spec")) ) { @@ -125,4 +125,6 @@ data_extract_spec <- function(dataname, select = NULL, filter = NULL, reshape = class = "data_extract_spec" ) } + assert_delayed_datesets(ans) + ans } diff --git a/R/delayed_datasets.R b/R/delayed_datasets.R index fb5e89fc..7abd3907 100644 --- a/R/delayed_datasets.R +++ b/R/delayed_datasets.R @@ -2,30 +2,49 @@ #' #' Generate `delayed_data_extract_spec` without prior knowledge of the data. #' -#' documentation WIP -#' #' `delayed_datasets` is a character string with class `delayed_datasets` -#' and attribute `datasets` which is set to `x`. -#' The attribute specifies a wishlist of datasets for which `delayed_des` are to be created. +#' and attribute `datasets`, which is set to `x`. The attribute specifies +#' a wishlist of datasets for which `delayed_data_extract_spec`s are to be created, +#' maintaining the same specification for `select`, `filter`, and `reshape`. +#' +#' `delayed_data_extract_spec` that have `delayed_datasets` for `dataname` are resolved internally. #' -#' `delayed_data_extract_specs` are resolved as follows: -#' - `data_extract_specs` are returned as is -#' - `delayed_data_extract_specs` where `dataname` is `character` are returned as is -#' - `delayed_data_extract_specs` where `dataname` is `delayed_datasets` is first confronted -#' with names of datasets in the app and has its `datasets` attribute updated, -#' and then is converted to a list of `delayed_data_extract_spec`s of the same length as -#' the updated `datasets` attribute. +#' It is forbidden to use different `delayed_datasets` within one `delayed_data_extract_spec` +#' as well as to mix `delayed_datasets` with specific dataset specification within one `delayed_data_extract_spec`. +#' This is enforced when creating `data_extract_spec`s. +#' +#' @inheritSection resolve_delayed_datasets #' #' @param x (`character`) set of dataset names for wchich `delayed_data_extract_spec`s will be created; #' set to `"all"` to use all available datasets -#' @param des (`data_extract_spec` or `list` thereof) see `Details` -#' @param datasets (`character`) vector of dataset for which to resolve - -#' @name delayed_datasets -NULL - -#' @rdname delayed_datasets +#' +#' @return Character string with `class` `delayed_datasets` and attribute `datasets`. +#' +#' @examples +#' # resolve into delayed_data_extract_specs for all available datasets +#' data_extract_spec( +#' dataname = delayed_datasets() +#' ) +#' +#' # resolve into delayed_data_extract_specs for available datasets from among ADSL and ADAE +#' data_extract_spec( +#' dataname = delayed_datasets(c("ADSL", "ADAE")) +#' ) +#' +#' # use the same delayed_datasets() in child elements of a des +#' data_extract_spec( +#' dataname = delayed_datasets(), +#' select = select_spec( +#' choices = variable_choices( +#' data = delayed_datasets(), +#' subset = function(data) names(Filter(is.numeric, data)) +#' ), +#' selected = last_choice() +#' ) +#' ) +#' #' @export +#' delayed_datasets <- function(x = "all") { structure( "delayed_datasets", @@ -33,83 +52,3 @@ delayed_datasets <- function(x = "all") { datasets = x ) } - -#' @rdname delayed_datasets -#' @export -resolve_delayed_datasets <- function(des, datasets) { - # When used on a ddes with delayed_dataset that is in a list - # .unfold_delayed_datasets creates a list(list(ddes, ddes)) structure - # where list(ddes, ddes) is expected. One list level has to be collapsed. - .integrate <- function(x) { - if (inherits(x, "delayed_data_extract_spec")) return(x) - if (checkmate::test_list(x, "list", len = 1L) && - checkmate::test_list(x[[1L]], "delayed_data_extract_spec")) { - return(x[[1L]]) - } - lapply(x, .integrate) - } - - .unfold_delayed_datasets(des, datasets) |> - .resolve_delayed_datasets() |> - .integrate() -} - -#' @keywords internal -#' @noRd -.unfold_delayed_datasets <- function(des, datasets) { - .horse <- function(des, datasets) { - delayed <- attr(des, "datasets", exact = TRUE) - delayed <- - if (identical(delayed, "all")) { - datasets - } else { - intersect(delayed, datasets) - } - attr(des, "datasets") <- delayed - des - } - - rapply(des, .horse, "delayed_datasets", how = "replace", datasets = datasets) -} - -#' @keywords internal -#' @noRd -.resolve_delayed_datasets <- function(des) { - .horse <- function(des) { - if (!inherits(des$dataname, "delayed_datasets")) return(des) - lapply(attr(des$dataname, "datasets", exact = TRUE), function(dataset) { - rapply(des, f = function(...) dataset, "delayed_datasets", how = "replace") - }) - } - - if (inherits(des, "delayed_data_extract_spec")) return(.horse(des)) - lapply(des, .resolve_delayed_datasets) -} - -#' ensure that all delayed_datasets in a delayed_des are the same -assert_delayed_datesets_identical <- function(x) { - checkmate::assert_class(x, "data_extract_spec") - if (inherits(x, "delayed_data_extract_spec")) { - master <- x$dataname - if (inherits(master, "delayed_datasets")) { - error_msg <- paste0(deparse1(match.call()), ": delayed_datasets identity violated") - slaves <- rapply(x, function(xx) xx, "delayed_datasets", how = "unlist") - slaves_datasets <- rapply(x, function(xx) attr(xx, "datasets"), "delayed_datasets", how = "unlist") - Reduce( - function(x1, x2) { - if (identical(x1, x2)) x2 else stop(error_msg, call. = FALSE) - }, - slaves, - init = as.vector(master) - ) - Reduce( - function(x1, x2) { - if (identical(x1, x2)) x2 else stop(error_msg, call. = FALSE) - }, - slaves_datasets, - init = attr(master, "datasets") - ) - } - } - x -} diff --git a/R/resolve_delayed_datasets.R b/R/resolve_delayed_datasets.R new file mode 100644 index 00000000..3fcf7a69 --- /dev/null +++ b/R/resolve_delayed_datasets.R @@ -0,0 +1,124 @@ +#' Resolve `delayed_datasets` +#' +#' Convert `delayed_data_extract_spec`s containing `delayed_datasets` into normal ones. +#' +#' This function is used internally. +#' +#' @param des (`data_extract_spec` or `list` thereof) see `Details` +#' @param datasets (`character`) vector of dataset for which to resolve +#' +#' @section Resolution: +#' `delayed_data_extract_specs` are resolved as follows: +#' - `data_extract_specs` are returned as is +#' - `delayed_data_extract_specs` where `dataname` is `character` are returned as is +#' - `delayed_data_extract_specs` where `dataname` is `delayed_datasets` is first confronted +#' with names of datasets in the app and has its `datasets` attribute updated, +#' and then is converted to a list of `delayed_data_extract_spec`s of the same length as +#' the updated `datasets` attribute. +#' +#' @return List of `delayed_data_extract_spec`s. +#' +#' @keywords internal +resolve_delayed_datasets <- function(des, datasets) { + # When used on a ddes with delayed_dataset that is in a list + # .unfold_delayed_datasets creates a list(list(ddes, ddes)) structure + # where list(ddes, ddes) is expected. One list level has to be collapsed. + .integrate <- function(x) { + if (inherits(x, "delayed_data_extract_spec")) { + return(x) + } + if (checkmate::test_list(x, "list", len = 1L) && + checkmate::test_list(x[[1L]], "delayed_data_extract_spec")) { + return(x[[1L]]) + } + lapply(x, .integrate) + } + + .unfold_delayed_datasets(des, datasets) |> + .resolve_delayed_datasets() |> + .integrate() +} + +#' @keywords internal +#' @noRd +.unfold_delayed_datasets <- function(des, datasets) { + .horse <- function(des, datasets) { + delayed <- attr(des, "datasets", exact = TRUE) + delayed <- + if (identical(delayed, "all")) { + datasets + } else { + intersect(delayed, datasets) + } + attr(des, "datasets") <- delayed + des + } + + rapply(des, .horse, "delayed_datasets", how = "replace", datasets = datasets) +} + +#' @keywords internal +#' @noRd +.resolve_delayed_datasets <- function(des) { + .horse <- function(des) { + if (!inherits(des$dataname, "delayed_datasets")) { + return(des) + } + lapply(attr(des$dataname, "datasets", exact = TRUE), function(dataset) { + rapply(des, f = function(...) dataset, "delayed_datasets", how = "replace") + }) + } + + if (inherits(des, "delayed_data_extract_spec")) { + return(.horse(des)) + } + lapply(des, .resolve_delayed_datasets) +} + +#' Assert delayed_datasets are used properly: +#' - no mixing with specific dataset specification +#' - no mixing different delayed_datasets +#' @keywords internal +#' @noRd +assert_delayed_datesets <- function(x) { + checkmate::assert_class(x, "data_extract_spec") + if (inherits(x, "delayed_data_extract_spec")) { + # STEP 1: check that all places that could be delayed_datasets are actually datasets + error_msg <- paste0(deparse1(sys.call(-1)), ": delayed_datasets must not be mixed with specific datanames") + .extract <- function(x) { + if (is.null(x) || is.logical(x) || is.function(x) || is.character(x)) return(NULL) + # Partial matching extracts both "data" and "dataname" + if (is.list(x) && is.character(x$data) && !inherits(x$data, "delayed_datasets")) return(x$data) + lapply(x, .extract) + } + datanames <- unlist(.extract(x)) + delayed <- vapply(datanames, inherits, logical(1L), what = "delayed_datasets") + if (!(all(delayed) || all(!delayed))) stop(error_msg, call. = FALSE) + + # STEP 2: check that all delayed_datasets in this ddes are the same + master <- x$dataname + if (inherits(master, "delayed_datasets")) { + error_msg <- paste0(deparse1(sys.call(-1)), ": delayed_datasets used must be identical") + slaves <- rapply(x, function(xx) xx, "delayed_datasets", how = "unlist") + slaves_datasets <- rapply(x, function(xx) attr(xx, "datasets"), "delayed_datasets", how = "unlist") + Reduce( + function(x1, x2) { + if (identical(x1, x2)) x2 else stop(error_msg, call. = FALSE) + }, + slaves, + init = as.vector(master) + ) + Reduce( + function(x1, x2) { + if (identical(x1, x2)) x2 else stop(error_msg, call. = FALSE) + }, + slaves_datasets, + init = attr(master, "datasets") + ) + } else { + error_msg <- paste0(deparse1(sys.call(-1)), ": delayed_datasets must not be mixed with specific datanames") + rapply(x, function(...) stop(error_msg, call. = FALSE), "delayed_datasets") + } + } + x +} From 2fec9aaf98ed72ba19a877ae34952cbd5dbaa38b Mon Sep 17 00:00:00 2001 From: Aleksander Chlebowski Date: Wed, 12 Feb 2025 16:28:21 +0100 Subject: [PATCH 09/17] more explicit argument checks in data_extract_spec --- R/data_extract_spec.R | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/R/data_extract_spec.R b/R/data_extract_spec.R index 71b8f5fe..6eef7e8b 100644 --- a/R/data_extract_spec.R +++ b/R/data_extract_spec.R @@ -84,7 +84,10 @@ #' @export #' data_extract_spec <- function(dataname, select = NULL, filter = NULL, reshape = FALSE) { - checkmate::assert_string(dataname) + checkmate::assert( + checkmate::check_string(dataname), + checkmate::check_class(dataname, "delayed_datasets") + ) stopifnot( is.null(select) || (inherits(select, "select_spec") && length(select) >= 1) From 97413b8a220dcd2a125ef227deb77cc9734fe47a Mon Sep 17 00:00:00 2001 From: Aleksander Chlebowski Date: Wed, 12 Feb 2025 16:33:19 +0100 Subject: [PATCH 10/17] amend documentation --- NAMESPACE | 1 + R/delayed_datasets.R | 2 +- man/delayed_datasets.Rd | 67 +++++++++++++++++++++++++++++++++ man/resolve_delayed_datasets.Rd | 36 ++++++++++++++++++ 4 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 man/delayed_datasets.Rd create mode 100644 man/resolve_delayed_datasets.Rd diff --git a/NAMESPACE b/NAMESPACE index 209b1855..74944206 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -45,6 +45,7 @@ export(data_extract_spec) export(data_extract_srv) export(data_extract_ui) export(datanames_input) +export(delayed_datasets) export(filter_spec) export(first_choice) export(first_choices) diff --git a/R/delayed_datasets.R b/R/delayed_datasets.R index 7abd3907..7591393a 100644 --- a/R/delayed_datasets.R +++ b/R/delayed_datasets.R @@ -13,7 +13,7 @@ #' as well as to mix `delayed_datasets` with specific dataset specification within one `delayed_data_extract_spec`. #' This is enforced when creating `data_extract_spec`s. #' -#' @inheritSection resolve_delayed_datasets +#' @inheritSection resolve_delayed_datasets Resolution #' #' @param x (`character`) set of dataset names for wchich `delayed_data_extract_spec`s will be created; #' set to `"all"` to use all available datasets diff --git a/man/delayed_datasets.Rd b/man/delayed_datasets.Rd new file mode 100644 index 00000000..0095da9f --- /dev/null +++ b/man/delayed_datasets.Rd @@ -0,0 +1,67 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/delayed_datasets.R +\name{delayed_datasets} +\alias{delayed_datasets} +\title{Delayed datasets} +\usage{ +delayed_datasets(x = "all") +} +\arguments{ +\item{x}{(\code{character}) set of dataset names for wchich \code{delayed_data_extract_spec}s will be created; +set to \code{"all"} to use all available datasets} +} +\value{ +Character string with \code{class} \code{delayed_datasets} and attribute \code{datasets}. +} +\description{ +Generate \code{delayed_data_extract_spec} without prior knowledge of the data. +} +\details{ +\code{delayed_datasets} is a character string with class \code{delayed_datasets} +and attribute \code{datasets}, which is set to \code{x}. The attribute specifies +a wishlist of datasets for which \code{delayed_data_extract_spec}s are to be created, +maintaining the same specification for \code{select}, \code{filter}, and \code{reshape}. + +\code{delayed_data_extract_spec} that have \code{delayed_datasets} for \code{dataname} are resolved internally. + +It is forbidden to use different \code{delayed_datasets} within one \code{delayed_data_extract_spec} +as well as to mix \code{delayed_datasets} with specific dataset specification within one \code{delayed_data_extract_spec}. +This is enforced when creating \code{data_extract_spec}s. +} +\section{Resolution}{ + +\code{delayed_data_extract_specs} are resolved as follows: +\itemize{ +\item \code{data_extract_specs} are returned as is +\item \code{delayed_data_extract_specs} where \code{dataname} is \code{character} are returned as is +\item \code{delayed_data_extract_specs} where \code{dataname} is \code{delayed_datasets} is first confronted +with names of datasets in the app and has its \code{datasets} attribute updated, +and then is converted to a list of \code{delayed_data_extract_spec}s of the same length as +the updated \code{datasets} attribute. +} +} + +\examples{ +# resolve into delayed_data_extract_specs for all available datasets +data_extract_spec( + dataname = delayed_datasets() +) + +# resolve into delayed_data_extract_specs for available datasets from among ADSL and ADAE +data_extract_spec( + dataname = delayed_datasets(c("ADSL", "ADAE")) +) + +# use the same delayed_datasets() in child elements of a des +data_extract_spec( + dataname = delayed_datasets(), + select = select_spec( + choices = variable_choices( + data = delayed_datasets(), + subset = function(data) names(Filter(is.numeric, data)) + ), + selected = last_choice() + ) +) + +} diff --git a/man/resolve_delayed_datasets.Rd b/man/resolve_delayed_datasets.Rd new file mode 100644 index 00000000..5587da01 --- /dev/null +++ b/man/resolve_delayed_datasets.Rd @@ -0,0 +1,36 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/resolve_delayed_datasets.R +\name{resolve_delayed_datasets} +\alias{resolve_delayed_datasets} +\title{Resolve \code{delayed_datasets}} +\usage{ +resolve_delayed_datasets(des, datasets) +} +\arguments{ +\item{des}{(\code{data_extract_spec} or \code{list} thereof) see \code{Details}} + +\item{datasets}{(\code{character}) vector of dataset for which to resolve} +} +\value{ +List of \code{delayed_data_extract_spec}s. +} +\description{ +Convert \code{delayed_data_extract_spec}s containing \code{delayed_datasets} into normal ones. +} +\details{ +This function is used internally. +} +\section{Resolution}{ + +\code{delayed_data_extract_specs} are resolved as follows: +\itemize{ +\item \code{data_extract_specs} are returned as is +\item \code{delayed_data_extract_specs} where \code{dataname} is \code{character} are returned as is +\item \code{delayed_data_extract_specs} where \code{dataname} is \code{delayed_datasets} is first confronted +with names of datasets in the app and has its \code{datasets} attribute updated, +and then is converted to a list of \code{delayed_data_extract_spec}s of the same length as +the updated \code{datasets} attribute. +} +} + +\keyword{internal} From 69b6de0215596c91c67eb16dee7918c939b3186d Mon Sep 17 00:00:00 2001 From: Aleksander Chlebowski Date: Wed, 12 Feb 2025 16:33:28 +0100 Subject: [PATCH 11/17] amend NEWS --- NEWS.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/NEWS.md b/NEWS.md index 0893897f..e55d6503 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,9 @@ # teal.transform 0.6.0.9000 +### Enhancements + +* Added utility function `delayed_datasets` that facilitates creating multiple `delayed_data_extract_spec`s without knowlege of the available datasets. It is now possible to create `delayed_data_extract_spec` for all available datasets with one call, rather than with one call per dataset. + # teal.transform 0.6.0 ### Enhancements From 2c22756ad1cd490bd7f37c8597ac1e86560e0da9 Mon Sep 17 00:00:00 2001 From: Aleksander Chlebowski Date: Wed, 12 Feb 2025 16:53:10 +0100 Subject: [PATCH 12/17] fix bug in assert_delayed_datasets --- R/resolve_delayed_datasets.R | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/R/resolve_delayed_datasets.R b/R/resolve_delayed_datasets.R index 3fcf7a69..c9b82a6a 100644 --- a/R/resolve_delayed_datasets.R +++ b/R/resolve_delayed_datasets.R @@ -100,7 +100,8 @@ assert_delayed_datesets <- function(x) { if (inherits(master, "delayed_datasets")) { error_msg <- paste0(deparse1(sys.call(-1)), ": delayed_datasets used must be identical") slaves <- rapply(x, function(xx) xx, "delayed_datasets", how = "unlist") - slaves_datasets <- rapply(x, function(xx) attr(xx, "datasets"), "delayed_datasets", how = "unlist") + .extract_datasets <- function(xx) paste(sort(attr(xx, "datasets")), collapse = "--") + slaves_datasets <- rapply(x, .extract_datasets, "delayed_datasets", how = "unlist") Reduce( function(x1, x2) { if (identical(x1, x2)) x2 else stop(error_msg, call. = FALSE) @@ -113,7 +114,7 @@ assert_delayed_datesets <- function(x) { if (identical(x1, x2)) x2 else stop(error_msg, call. = FALSE) }, slaves_datasets, - init = attr(master, "datasets") + init = .extract_datasets(master) ) } else { error_msg <- paste0(deparse1(sys.call(-1)), ": delayed_datasets must not be mixed with specific datanames") From cb5e366913a6a5d3b85d834ba936e2afbb29d805 Mon Sep 17 00:00:00 2001 From: Aleksander Chlebowski Date: Wed, 12 Feb 2025 17:06:22 +0100 Subject: [PATCH 13/17] remove parital matching b/c of testthat complaints --- R/resolve_delayed_datasets.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/R/resolve_delayed_datasets.R b/R/resolve_delayed_datasets.R index c9b82a6a..d05cb9d6 100644 --- a/R/resolve_delayed_datasets.R +++ b/R/resolve_delayed_datasets.R @@ -87,8 +87,8 @@ assert_delayed_datesets <- function(x) { error_msg <- paste0(deparse1(sys.call(-1)), ": delayed_datasets must not be mixed with specific datanames") .extract <- function(x) { if (is.null(x) || is.logical(x) || is.function(x) || is.character(x)) return(NULL) - # Partial matching extracts both "data" and "dataname" - if (is.list(x) && is.character(x$data) && !inherits(x$data, "delayed_datasets")) return(x$data) + if (is.list(x) && is.character(x[["data"]]) && !inherits(x[["data"]], "delayed_datasets")) return(x[["data"]]) + if (is.list(x) && is.character(x[["dataname"]]) && !inherits(x[["dataname"]], "delayed_datasets")) return(x[["dataname"]]) # nolint: line_length. lapply(x, .extract) } datanames <- unlist(.extract(x)) From 78580ea1b169a95159351484172467643b1326a7 Mon Sep 17 00:00:00 2001 From: Aleksander Chlebowski Date: Wed, 12 Feb 2025 17:14:29 +0100 Subject: [PATCH 14/17] add tests for data_extract_spec constructor with delayed_datasets --- tests/testthat/test-delayed_datasets.R | 52 ++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/tests/testthat/test-delayed_datasets.R b/tests/testthat/test-delayed_datasets.R index c6450708..820d8204 100644 --- a/tests/testthat/test-delayed_datasets.R +++ b/tests/testthat/test-delayed_datasets.R @@ -1,3 +1,55 @@ +testthat::test_that("delayed_data_extract_spec can be constructed with delayed_datasets", { + testthat::expect_no_error( + data_extract_spec( + dataname = delayed_datasets() + ) + ) + testthat::expect_no_error( + data_extract_spec( + dataname = delayed_datasets(), + select = select_spec( + choices = variable_choices( + data = delayed_datasets(), + subset = function(data) names(Filter(is.numeric, data)) + ), + selected = first_choice() + ) + ) + ) +}) + +testthat::test_that("delayed_data_extract_spec cannot be constructed with mixed delayed_datasets", { + testthat::expect_error( + data_extract_spec( + dataname = delayed_datasets("ADSL"), + select = select_spec( + choices = variable_choices( + data = delayed_datasets("ADAE"), + subset = function(data) names(Filter(is.numeric, data)) + ), + selected = first_choice() + ) + ), + "delayed_datasets used must be identical" + ) +}) + +testthat::test_that("delayed_data_extract_spec cannot be constructed with delayed_datasets mixed with specific datasets", { # nolint: line_length. + testthat::expect_error( + data_extract_spec( + dataname = "ADSL", + select = select_spec( + choices = variable_choices( + data = delayed_datasets("ADSL"), + subset = function(data) names(Filter(is.numeric, data)) + ), + selected = first_choice() + ) + ), + "delayed_datasets must not be mixed with specific datanames" + ) +}) + data <- teal.data::cdisc_data( ADSL = teal.data::rADSL, ADAE = teal.data::rADAE From 3308e76c979900fafc7631516a3360555159b82b Mon Sep 17 00:00:00 2001 From: Aleksander Chlebowski Date: Wed, 12 Feb 2025 22:14:08 +0100 Subject: [PATCH 15/17] clean up --- R/data_extract_module.R | 1 - 1 file changed, 1 deletion(-) diff --git a/R/data_extract_module.R b/R/data_extract_module.R index 8919fbaa..7eaf7ee5 100644 --- a/R/data_extract_module.R +++ b/R/data_extract_module.R @@ -537,7 +537,6 @@ data_extract_srv.list <- function(id, dataset_input <- if (isTRUE(input$is_single_dataset)) { - # if (FALSE) { NULL } else { if (length(dataset_names) == 1) { From a4bb6da2a19e090638d5edd14fa03b1391501077 Mon Sep 17 00:00:00 2001 From: Aleksander Chlebowski Date: Wed, 12 Feb 2025 22:30:56 +0100 Subject: [PATCH 16/17] update conditions in resolve_delayed_datasets to include non-delayed des --- R/resolve_delayed_datasets.R | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/R/resolve_delayed_datasets.R b/R/resolve_delayed_datasets.R index d05cb9d6..dc7fbc9b 100644 --- a/R/resolve_delayed_datasets.R +++ b/R/resolve_delayed_datasets.R @@ -24,11 +24,11 @@ resolve_delayed_datasets <- function(des, datasets) { # .unfold_delayed_datasets creates a list(list(ddes, ddes)) structure # where list(ddes, ddes) is expected. One list level has to be collapsed. .integrate <- function(x) { - if (inherits(x, "delayed_data_extract_spec")) { + if (inherits(x, "data_extract_spec")) { return(x) } if (checkmate::test_list(x, "list", len = 1L) && - checkmate::test_list(x[[1L]], "delayed_data_extract_spec")) { + checkmate::test_list(x[[1L]], "data_extract_spec")) { return(x[[1L]]) } lapply(x, .integrate) @@ -69,7 +69,7 @@ resolve_delayed_datasets <- function(des, datasets) { }) } - if (inherits(des, "delayed_data_extract_spec")) { + if (inherits(des, "data_extract_spec")) { return(.horse(des)) } lapply(des, .resolve_delayed_datasets) From 06c34ea7f3606ff4f19e55dcb8668caf7fec4a3b Mon Sep 17 00:00:00 2001 From: Aleksander Chlebowski Date: Thu, 13 Feb 2025 01:01:12 +0100 Subject: [PATCH 17/17] restyle to NEST standard --- R/resolve_delayed_datasets.R | 38 +++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/R/resolve_delayed_datasets.R b/R/resolve_delayed_datasets.R index dc7fbc9b..81574c65 100644 --- a/R/resolve_delayed_datasets.R +++ b/R/resolve_delayed_datasets.R @@ -25,13 +25,12 @@ resolve_delayed_datasets <- function(des, datasets) { # where list(ddes, ddes) is expected. One list level has to be collapsed. .integrate <- function(x) { if (inherits(x, "data_extract_spec")) { - return(x) - } - if (checkmate::test_list(x, "list", len = 1L) && - checkmate::test_list(x[[1L]], "data_extract_spec")) { - return(x[[1L]]) + x + } else if (checkmate::test_list(x, "list", len = 1L) && checkmate::test_list(x[[1L]], "data_extract_spec")) { + x[[1L]] + } else { + lapply(x, .integrate) } - lapply(x, .integrate) } .unfold_delayed_datasets(des, datasets) |> @@ -62,17 +61,19 @@ resolve_delayed_datasets <- function(des, datasets) { .resolve_delayed_datasets <- function(des) { .horse <- function(des) { if (!inherits(des$dataname, "delayed_datasets")) { - return(des) + des + } else { + lapply(attr(des$dataname, "datasets", exact = TRUE), function(dataset) { + rapply(des, f = function(...) dataset, "delayed_datasets", how = "replace") + }) } - lapply(attr(des$dataname, "datasets", exact = TRUE), function(dataset) { - rapply(des, f = function(...) dataset, "delayed_datasets", how = "replace") - }) } if (inherits(des, "data_extract_spec")) { - return(.horse(des)) + .horse(des) + } else { + lapply(des, .resolve_delayed_datasets) } - lapply(des, .resolve_delayed_datasets) } #' Assert delayed_datasets are used properly: @@ -86,10 +87,15 @@ assert_delayed_datesets <- function(x) { # STEP 1: check that all places that could be delayed_datasets are actually datasets error_msg <- paste0(deparse1(sys.call(-1)), ": delayed_datasets must not be mixed with specific datanames") .extract <- function(x) { - if (is.null(x) || is.logical(x) || is.function(x) || is.character(x)) return(NULL) - if (is.list(x) && is.character(x[["data"]]) && !inherits(x[["data"]], "delayed_datasets")) return(x[["data"]]) - if (is.list(x) && is.character(x[["dataname"]]) && !inherits(x[["dataname"]], "delayed_datasets")) return(x[["dataname"]]) # nolint: line_length. - lapply(x, .extract) + if (is.null(x) || is.logical(x) || is.function(x) || is.character(x)) { + NULL + } else if (is.list(x) && is.character(x[["data"]]) && !inherits(x[["data"]], "delayed_datasets")) { + x[["data"]] + } else if (is.list(x) && is.character(x[["dataname"]]) && !inherits(x[["dataname"]], "delayed_datasets")) { + x[["dataname"]] + } else { + lapply(x, .extract) + } } datanames <- unlist(.extract(x)) delayed <- vapply(datanames, inherits, logical(1L), what = "delayed_datasets")