From 3908349b1af92abadabb22c109b8251529a87e9d Mon Sep 17 00:00:00 2001 From: vedhav Date: Fri, 3 Nov 2023 20:09:39 +0530 Subject: [PATCH 01/15] fix: make join_keys related changes due to refactor --- R/data_extract_module.R | 2 +- R/get_merge_call.R | 2 +- R/merge_datasets.R | 2 +- tests/testthat/test-merge_expression_srv.R | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/R/data_extract_module.R b/R/data_extract_module.R index e21cd242..19707e68 100644 --- a/R/data_extract_module.R +++ b/R/data_extract_module.R @@ -457,7 +457,7 @@ data_extract_srv.list <- function(id, datasets, data_extract_spec, join_keys = N # get keys out of join_keys if (!is.null(join_keys)) { - keys <- sapply(names(datasets), simplify = FALSE, function(x) join_keys$get(x, x)) + keys <- sapply(names(datasets), simplify = FALSE, function(x) join_keys[x, x]) } else { keys <- sapply(names(datasets), simplify = FALSE, function(x) character(0)) } diff --git a/R/get_merge_call.R b/R/get_merge_call.R index 35230eec..1b22ed1d 100644 --- a/R/get_merge_call.R +++ b/R/get_merge_call.R @@ -130,7 +130,7 @@ get_merge_key_grid <- function(selector_list, join_keys = teal.data::join_keys() get_merge_key_pair( selector_from, selector_to, - join_keys$get(selector_from$dataname, selector_to$dataname) + join_keys[selector_from$dataname, selector_to$dataname] ) } ) diff --git a/R/merge_datasets.R b/R/merge_datasets.R index 83199d70..3ff60973 100644 --- a/R/merge_datasets.R +++ b/R/merge_datasets.R @@ -278,7 +278,7 @@ are_needed_keys_provided <- function(join_keys, merged_selector_list) { } do_join_keys_exist <- function(dataset_name1, dataset_name2, join_keys) { - length(join_keys$get(dataset_name1, dataset_name2) > 0) + length(join_keys[dataset_name1, dataset_name2] > 0) } datasets_names <- vapply(merged_selector_list, function(slice) slice[["dataname"]], FUN.VALUE = character(1)) diff --git a/tests/testthat/test-merge_expression_srv.R b/tests/testthat/test-merge_expression_srv.R index e4d4057a..13212421 100644 --- a/tests/testthat/test-merge_expression_srv.R +++ b/tests/testthat/test-merge_expression_srv.R @@ -20,7 +20,7 @@ adsl_data_extract_srv_output <- dataname = "ADSL", filters = NULL, select = "AGE", - keys = join_keys$get("ADSL", "ADSL"), + keys = join_keys["ADSL", "ADSL"], reshape = FALSE, internal_id = "adsl_extract" ) @@ -30,7 +30,7 @@ adlb_data_extract_srv_output <- dataname = "ADLB", filters = NULL, select = c("AVAL", "CHG"), - keys = join_keys$get("ADLB", "ADLB"), + keys = join_keys["ADLB", "ADLB"], reshape = FALSE, internal_id = "adlb_extract" ) From 37e5df3df5aa04b30e5ebd94d3d549dd72fa831d Mon Sep 17 00:00:00 2001 From: vedhav Date: Mon, 6 Nov 2023 11:50:02 +0530 Subject: [PATCH 02/15] chore: fix broken tests --- R/choices_labeled.R | 2 +- R/data_extract_module.R | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/R/choices_labeled.R b/R/choices_labeled.R index ea1b2c7d..93fcc2c7 100644 --- a/R/choices_labeled.R +++ b/R/choices_labeled.R @@ -378,7 +378,7 @@ value_choices.data.frame <- function(data, # nolint df_choices <- dplyr::mutate_if( df_choices, .predicate = function(col) inherits(col, c("POSIXct", "POSIXlt", "POSIXt")), - .fun = function(col) { + .funs = function(col) { if (is.null(attr(col, "tzone")) || all(attr(col, "tzone") == "")) { format(trunc(col), "%Y-%m-%d %H:%M:%S") } else { diff --git a/R/data_extract_module.R b/R/data_extract_module.R index 19707e68..09285786 100644 --- a/R/data_extract_module.R +++ b/R/data_extract_module.R @@ -704,15 +704,15 @@ data_extract_multiple_srv.list <- function(data_extract, datasets, join_keys = N checkmate::assert_list(datasets, types = c("reactive", "data.frame"), names = "named") checkmate::assert_class(join_keys, "JoinKeys", null.ok = TRUE) checkmate::assert( - checkmate::check_multi_class(select_validation_rule, class = c("function", "formula"), null.ok = TRUE), + checkmate::check_multi_class(select_validation_rule, classes = c("function", "formula"), null.ok = TRUE), checkmate::check_list(select_validation_rule, types = c("function", "formula", "NULL"), null.ok = TRUE) ) checkmate::assert( - checkmate::check_multi_class(filter_validation_rule, class = c("function", "formula"), null.ok = TRUE), + checkmate::check_multi_class(filter_validation_rule, classes = c("function", "formula"), null.ok = TRUE), checkmate::check_list(filter_validation_rule, types = c("function", "formula", "NULL"), null.ok = TRUE) ) checkmate::assert( - checkmate::check_multi_class(dataset_validation_rule, class = c("function", "formula"), null.ok = TRUE), + checkmate::check_multi_class(dataset_validation_rule, classes = c("function", "formula"), null.ok = TRUE), checkmate::check_list(dataset_validation_rule, types = c("function", "formula", "NULL"), null.ok = TRUE) ) From b2c1781292981daffcda6f50f50b240f020634a9 Mon Sep 17 00:00:00 2001 From: vedhav Date: Mon, 6 Nov 2023 18:37:36 +0530 Subject: [PATCH 03/15] chore: fix partial arg matches --- R/resolve.R | 2 +- R/resolve_delayed.R | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/R/resolve.R b/R/resolve.R index 6a053473..afd4607e 100644 --- a/R/resolve.R +++ b/R/resolve.R @@ -64,7 +64,7 @@ #' teal.transform:::resolve(arm_ref_comp, data_list, keys) #' }) resolve <- function(x, datasets, keys = NULL) { - checkmate::assert_list(datasets, type = "reactive", min.len = 1, names = "named") + checkmate::assert_list(datasets, types = "reactive", min.len = 1, names = "named") checkmate::assert_list(keys, "character", names = "named", null.ok = TRUE) checkmate::assert( .var.name = "keys", diff --git a/R/resolve_delayed.R b/R/resolve_delayed.R index 3aca5bb9..74f361f4 100644 --- a/R/resolve_delayed.R +++ b/R/resolve_delayed.R @@ -83,7 +83,7 @@ resolve_delayed.FilteredData <- function(x, #' @export resolve_delayed.list <- function(x, datasets, keys = NULL) { - checkmate::assert_list(datasets, type = c("reactive", "data.frame"), min.len = 1, names = "named") + checkmate::assert_list(datasets, types = c("reactive", "data.frame"), min.len = 1, names = "named") checkmate::assert_list(keys, "character", names = "named", null.ok = TRUE) checkmate::assert( .var.name = "keys", From 9cf56daa9e76d239e6547d9a7ede4e839facadf9 Mon Sep 17 00:00:00 2001 From: vedhav Date: Tue, 7 Nov 2023 14:58:37 +0530 Subject: [PATCH 04/15] chore: replace '[' with '[[' --- R/data_extract_module.R | 2 +- R/get_merge_call.R | 2 +- R/merge_datasets.R | 2 +- tests/testthat/test-merge_expression_srv.R | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/R/data_extract_module.R b/R/data_extract_module.R index 09285786..1ff4e149 100644 --- a/R/data_extract_module.R +++ b/R/data_extract_module.R @@ -457,7 +457,7 @@ data_extract_srv.list <- function(id, datasets, data_extract_spec, join_keys = N # get keys out of join_keys if (!is.null(join_keys)) { - keys <- sapply(names(datasets), simplify = FALSE, function(x) join_keys[x, x]) + keys <- sapply(names(datasets), simplify = FALSE, function(x) join_keys[[x]][[x]]) } else { keys <- sapply(names(datasets), simplify = FALSE, function(x) character(0)) } diff --git a/R/get_merge_call.R b/R/get_merge_call.R index 1b22ed1d..4541b369 100644 --- a/R/get_merge_call.R +++ b/R/get_merge_call.R @@ -130,7 +130,7 @@ get_merge_key_grid <- function(selector_list, join_keys = teal.data::join_keys() get_merge_key_pair( selector_from, selector_to, - join_keys[selector_from$dataname, selector_to$dataname] + join_keys[[selector_from$dataname]][[selector_to$dataname]] ) } ) diff --git a/R/merge_datasets.R b/R/merge_datasets.R index 3ff60973..87fd3b76 100644 --- a/R/merge_datasets.R +++ b/R/merge_datasets.R @@ -278,7 +278,7 @@ are_needed_keys_provided <- function(join_keys, merged_selector_list) { } do_join_keys_exist <- function(dataset_name1, dataset_name2, join_keys) { - length(join_keys[dataset_name1, dataset_name2] > 0) + length(join_keys[[dataset_name1]][[dataset_name2]] > 0) } datasets_names <- vapply(merged_selector_list, function(slice) slice[["dataname"]], FUN.VALUE = character(1)) diff --git a/tests/testthat/test-merge_expression_srv.R b/tests/testthat/test-merge_expression_srv.R index 13212421..bc6b43f8 100644 --- a/tests/testthat/test-merge_expression_srv.R +++ b/tests/testthat/test-merge_expression_srv.R @@ -20,7 +20,7 @@ adsl_data_extract_srv_output <- dataname = "ADSL", filters = NULL, select = "AGE", - keys = join_keys["ADSL", "ADSL"], + keys = join_keys[["ADSL", "ADSL"]], reshape = FALSE, internal_id = "adsl_extract" ) @@ -30,7 +30,7 @@ adlb_data_extract_srv_output <- dataname = "ADLB", filters = NULL, select = c("AVAL", "CHG"), - keys = join_keys["ADLB", "ADLB"], + keys = join_keys[["ADLB", "ADLB"]], reshape = FALSE, internal_id = "adlb_extract" ) From 0d5d063b8fcccfcbe729bb1875aa98bd9aaff6b7 Mon Sep 17 00:00:00 2001 From: Vedha Viyash <49812166+vedhav@users.noreply.github.com> Date: Tue, 7 Nov 2023 19:14:27 +0530 Subject: [PATCH 05/15] Update tests/testthat/test-merge_expression_srv.R MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Dawid Kałędkowski <6959016+gogonzo@users.noreply.github.com> Signed-off-by: Vedha Viyash <49812166+vedhav@users.noreply.github.com> --- tests/testthat/test-merge_expression_srv.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testthat/test-merge_expression_srv.R b/tests/testthat/test-merge_expression_srv.R index bc6b43f8..2e922dde 100644 --- a/tests/testthat/test-merge_expression_srv.R +++ b/tests/testthat/test-merge_expression_srv.R @@ -30,7 +30,7 @@ adlb_data_extract_srv_output <- dataname = "ADLB", filters = NULL, select = c("AVAL", "CHG"), - keys = join_keys[["ADLB", "ADLB"]], + keys = join_keys[["ADLB"]][["ADLB"]], reshape = FALSE, internal_id = "adlb_extract" ) From f258ae1f3ac18a0e18d9341feb79feb488e9c774 Mon Sep 17 00:00:00 2001 From: Vedha Viyash <49812166+vedhav@users.noreply.github.com> Date: Tue, 7 Nov 2023 19:14:33 +0530 Subject: [PATCH 06/15] Update tests/testthat/test-merge_expression_srv.R MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Dawid Kałędkowski <6959016+gogonzo@users.noreply.github.com> Signed-off-by: Vedha Viyash <49812166+vedhav@users.noreply.github.com> --- tests/testthat/test-merge_expression_srv.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/testthat/test-merge_expression_srv.R b/tests/testthat/test-merge_expression_srv.R index 2e922dde..f6e03da7 100644 --- a/tests/testthat/test-merge_expression_srv.R +++ b/tests/testthat/test-merge_expression_srv.R @@ -20,7 +20,7 @@ adsl_data_extract_srv_output <- dataname = "ADSL", filters = NULL, select = "AGE", - keys = join_keys[["ADSL", "ADSL"]], + keys = join_keys[["ADSL"]][["ADSL"]], reshape = FALSE, internal_id = "adsl_extract" ) From ed7aa885dc65418370da0c8c48f3f71dd9aedfca Mon Sep 17 00:00:00 2001 From: vedhav Date: Wed, 8 Nov 2023 12:27:53 +0530 Subject: [PATCH 07/15] chore: fix lint errors --- R/merge_datasets.R | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/R/merge_datasets.R b/R/merge_datasets.R index 87fd3b76..109e1997 100644 --- a/R/merge_datasets.R +++ b/R/merge_datasets.R @@ -177,10 +177,12 @@ merge_selectors <- function(selector_list) { next } selector_idx2 <- selector_list[[idx2]] - if (identical(selector_idx1$dataname, selector_idx2$dataname) && - identical(selector_idx1$reshape, selector_idx2$reshape) && - identical(selector_idx1$filters, selector_idx2$filters) && - identical(selector_idx1$keys, selector_idx2$keys)) { + if ( + identical(selector_idx1$dataname, selector_idx2$dataname) && + identical(selector_idx1$reshape, selector_idx2$reshape) && + identical(selector_idx1$filters, selector_idx2$filters) && + identical(selector_idx1$keys, selector_idx2$keys) + ) { res_map_idx[idx2] <- idx1 } } From dd6a0035918a66391edad41e9feecce1b5c121fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Ver=C3=ADssimo?= <211358+averissimo@users.noreply.github.com> Date: Wed, 8 Nov 2023 15:13:30 +0100 Subject: [PATCH 08/15] Rename JoinKeys class to join_keys from upstream {teal.data} (#161) All tests pass with exception of: ![image](https://github.com/insightsengineering/teal.transform/assets/211358/bd40791c-bc17-45c4-984d-7f4902a192d5) --- R/data_extract_module.R | 8 ++++---- R/get_dplyr_call.R | 4 ++-- R/get_merge_call.R | 2 +- R/merge_datasets.R | 4 ++-- R/merge_expression_module.R | 6 +++--- man/are_needed_keys_provided.Rd | 2 +- man/data_extract_multiple_srv.Rd | 2 +- man/data_extract_srv.Rd | 2 +- man/get_dplyr_call.Rd | 2 +- man/get_dplyr_call_data.Rd | 2 +- man/get_merge_call.Rd | 2 +- man/get_merge_key_grid.Rd | 2 +- man/get_rename_call.Rd | 2 +- man/get_reshape_call.Rd | 2 +- man/merge_datasets.Rd | 2 +- man/merge_expression_module.Rd | 2 +- man/merge_expression_srv.Rd | 2 +- man/validate_keys_sufficient.Rd | 2 +- tests/testthat/test-data_extract_multiple_srv.R | 2 +- tests/testthat/test-data_extract_srv.R | 2 +- tests/testthat/test-merge_expression_srv.R | 4 ++-- 21 files changed, 29 insertions(+), 29 deletions(-) diff --git a/R/data_extract_module.R b/R/data_extract_module.R index 1ff4e149..4e924c48 100644 --- a/R/data_extract_module.R +++ b/R/data_extract_module.R @@ -421,7 +421,7 @@ data_extract_srv.FilteredData <- function(id, datasets, data_extract_spec, ...) } #' @rdname data_extract_srv -#' @param join_keys (`JoinKeys` or `NULL`) of keys per dataset in `datasets` +#' @param join_keys (`join_keys` or `NULL`) of keys per dataset in `datasets` #' @param select_validation_rule (`NULL` or `function`) #' Should there be any `shinyvalidate` input validation of the select parts of the `data_extract_ui`. #' You can use a validation function directly (i.e. `select_validation_rule = shinyvalidate::sv_required()`) @@ -443,7 +443,7 @@ data_extract_srv.list <- function(id, datasets, data_extract_spec, join_keys = N }, ...) { checkmate::assert_list(datasets, types = c("reactive", "data.frame"), names = "named") - checkmate::assert_class(join_keys, "JoinKeys", null.ok = TRUE) + checkmate::assert_class(join_keys, "join_keys", null.ok = TRUE) checkmate::assert_multi_class(select_validation_rule, classes = c("function", "formula"), null.ok = TRUE) checkmate::assert_multi_class(filter_validation_rule, classes = c("function", "formula"), null.ok = TRUE) checkmate::assert_multi_class(dataset_validation_rule, classes = c("function", "formula"), null.ok = TRUE) @@ -681,7 +681,7 @@ data_extract_multiple_srv.FilteredData <- function(data_extract, datasets, ...) } #' @rdname data_extract_multiple_srv -#' @param join_keys (`JoinKeys` or `NULL`) of join keys per dataset in `datasets`. +#' @param join_keys (`join_keys` or `NULL`) of join keys per dataset in `datasets`. #' @param select_validation_rule (`NULL`, `function` or `named list` of `function`) #' Should there be any `shinyvalidate` input validation of the select parts of the `data_extract_ui` #' If all `data_extract` require the same validation function then this can be used directly ( @@ -702,7 +702,7 @@ data_extract_multiple_srv.list <- function(data_extract, datasets, join_keys = N shinyvalidate::sv_required("Please select a dataset") }, ...) { checkmate::assert_list(datasets, types = c("reactive", "data.frame"), names = "named") - checkmate::assert_class(join_keys, "JoinKeys", null.ok = TRUE) + checkmate::assert_class(join_keys, "join_keys", null.ok = TRUE) checkmate::assert( checkmate::check_multi_class(select_validation_rule, classes = c("function", "formula"), null.ok = TRUE), checkmate::check_list(select_validation_rule, types = c("function", "formula", "NULL"), null.ok = TRUE) diff --git a/R/get_dplyr_call.R b/R/get_dplyr_call.R index c277a66a..e0724100 100644 --- a/R/get_dplyr_call.R +++ b/R/get_dplyr_call.R @@ -9,7 +9,7 @@ #' @keywords internal get_dplyr_call_data <- function(selector_list, join_keys = teal.data::join_keys()) { logger::log_trace("get_dplyr_call_data called with: { paste(names(selector_list), collapse = ', ') } selectors.") - checkmate::assert_class(join_keys, "JoinKeys") + checkmate::assert_class(join_keys, "join_keys") lapply(selector_list, check_selector) all_merge_key_list <- get_merge_key_grid(selector_list, join_keys) @@ -211,7 +211,7 @@ get_dplyr_call <- function(selector_list, ) ) lapply(selector_list, check_selector) - checkmate::assert_class(join_keys, "JoinKeys", null.ok = TRUE) + checkmate::assert_class(join_keys, "join_keys", null.ok = TRUE) checkmate::assert_integer(idx, len = 1, any.missing = FALSE) n_selectors <- length(selector_list) diff --git a/R/get_merge_call.R b/R/get_merge_call.R index 4541b369..dd7ad272 100644 --- a/R/get_merge_call.R +++ b/R/get_merge_call.R @@ -4,7 +4,7 @@ #' Returns list of calls depending on selector(s) and type of the merge #' Order of merge is the same as in selectors passed to the function. #' @inheritParams merge_datasets -#' @param join_keys (`JoinKeys`) nested list of keys used for joining +#' @param join_keys (`join_keys`) nested list of keys used for joining #' @param dplyr_call_data (`list`) simplified selectors with aggregated set of filters, #' #' @return (`list` with `call` elements) diff --git a/R/merge_datasets.R b/R/merge_datasets.R index 109e1997..98fcc79c 100644 --- a/R/merge_datasets.R +++ b/R/merge_datasets.R @@ -51,7 +51,7 @@ merge_datasets <- function(selector_list, datasets, join_keys, merge_function = checkmate::assert_list(selector_list, min.len = 1) checkmate::assert_string(anl_name) checkmate::assert_list(datasets, names = "named") - checkmate::assert_class(join_keys, "JoinKeys") + checkmate::assert_class(join_keys, "join_keys") stopifnot(attr(regexec("[A-Za-z0-9\\_]*", anl_name)[[1]], "match.length") == nchar(anl_name)) lapply(selector_list, check_selector) merge_selectors_out <- merge_selectors(selector_list) @@ -241,7 +241,7 @@ check_data_merge_selectors <- function(selector_list) { #' `merged_selector_list` come from datasets, which don't have the #' appropriate join keys in `join_keys`. #' -#' @param join_keys (`JoinKeys`) the provided join keys +#' @param join_keys (`join_keys`) the provided join keys #' @param merged_selector_list (`list`) the specification of datasets' slices to merge #' #' @return `TRUE` if the provided keys meet the requirements; the `shiny` diff --git a/R/merge_expression_module.R b/R/merge_expression_module.R index 1906806a..7dd518f0 100644 --- a/R/merge_expression_module.R +++ b/R/merge_expression_module.R @@ -9,7 +9,7 @@ #' @param datasets (named `list` of `reactive` or non-`reactive` `data.frame`)\cr #' object containing data as a list of `data.frame`. When passing a list of non-reactive `data.frame` objects, they are #' converted to reactive `data.frame` objects internally. -#' @param join_keys (`JoinKeys`)\cr +#' @param join_keys (`join_keys`)\cr #' of variables used as join keys for each of the datasets in `datasets`. #' This will be used to extract the `keys` of every dataset. #' @param data_extract (named `list` of `data_extract_spec`)\cr @@ -171,7 +171,7 @@ merge_expression_module <- function(datasets, #' @param datasets (named `list` of `reactive` or non-`reactive` `data.frame`)\cr #' object containing data as a list of `data.frame`. When passing a list of non-reactive `data.frame` objects, they are #' converted to reactive `data.frame` objects internally. -#' @param join_keys (`JoinKeys`)\cr +#' @param join_keys (`join_keys`)\cr #' of variables used as join keys for each of the datasets in `datasets`. #' This will be used to extract the `keys` of every dataset. #' @param selector_list (`reactive`)\cr @@ -315,7 +315,7 @@ merge_expression_srv <- function(id = "merge_id", stopifnot(make.names(anl_name) == anl_name) checkmate::assert_class(selector_list, "reactive") checkmate::assert_list(datasets, types = c("reactive", "data.frame"), names = "named") - checkmate::assert_class(join_keys, "JoinKeys") + checkmate::assert_class(join_keys, "join_keys") moduleServer( id, diff --git a/man/are_needed_keys_provided.Rd b/man/are_needed_keys_provided.Rd index f98b6143..51730568 100644 --- a/man/are_needed_keys_provided.Rd +++ b/man/are_needed_keys_provided.Rd @@ -7,7 +7,7 @@ are_needed_keys_provided(join_keys, merged_selector_list) } \arguments{ -\item{join_keys}{(\code{JoinKeys}) the provided join keys} +\item{join_keys}{(\code{join_keys}) the provided join keys} \item{merged_selector_list}{(\code{list}) the specification of datasets' slices to merge} } diff --git a/man/data_extract_multiple_srv.Rd b/man/data_extract_multiple_srv.Rd index 5fcb7f54..fe52d972 100644 --- a/man/data_extract_multiple_srv.Rd +++ b/man/data_extract_multiple_srv.Rd @@ -39,7 +39,7 @@ When passing a list of reactive or non-reactive \code{data.frame} objects, the a \item{...}{an additional argument \code{join_keys} is required when \code{datasets} is a list of \code{data.frame}. It shall contain the keys per dataset in \code{datasets}.} -\item{join_keys}{(\code{JoinKeys} or \code{NULL}) of join keys per dataset in \code{datasets}.} +\item{join_keys}{(\code{join_keys} or \code{NULL}) of join keys per dataset in \code{datasets}.} \item{select_validation_rule}{(\code{NULL}, \code{function} or \verb{named list} of \code{function}) Should there be any \code{shinyvalidate} input validation of the select parts of the \code{data_extract_ui} diff --git a/man/data_extract_srv.Rd b/man/data_extract_srv.Rd index d0fef69b..376d1264 100644 --- a/man/data_extract_srv.Rd +++ b/man/data_extract_srv.Rd @@ -42,7 +42,7 @@ A list of data filter and select information constructed by \link{data_extract_s \item{...}{an additional argument \code{join_keys} is required when \code{datasets} is a list of \code{data.frame}. It shall contain the keys per dataset in \code{datasets}.} -\item{join_keys}{(\code{JoinKeys} or \code{NULL}) of keys per dataset in \code{datasets}} +\item{join_keys}{(\code{join_keys} or \code{NULL}) of keys per dataset in \code{datasets}} \item{select_validation_rule}{(\code{NULL} or \code{function}) Should there be any \code{shinyvalidate} input validation of the select parts of the \code{data_extract_ui}. diff --git a/man/get_dplyr_call.Rd b/man/get_dplyr_call.Rd index 87b193d0..ce688c49 100644 --- a/man/get_dplyr_call.Rd +++ b/man/get_dplyr_call.Rd @@ -20,7 +20,7 @@ When using a reactive named list, the names must be identical to the shiny ids o \item{idx}{optional (\code{integer}) current selector index in all selectors list} -\item{join_keys}{(\code{JoinKeys}) nested list of keys used for joining} +\item{join_keys}{(\code{join_keys}) nested list of keys used for joining} \item{dplyr_call_data}{(\code{list}) simplified selectors with aggregated set of filters, selections, reshapes etc. All necessary data for merging} diff --git a/man/get_dplyr_call_data.Rd b/man/get_dplyr_call_data.Rd index 82cead4f..f85b831d 100644 --- a/man/get_dplyr_call_data.Rd +++ b/man/get_dplyr_call_data.Rd @@ -12,7 +12,7 @@ output from \code{\link[=data_extract_multiple_srv]{data_extract_multiple_srv()} When using a reactive named list, the names must be identical to the shiny ids of the respective \code{\link[=data_extract_ui]{data_extract_ui()}}.} -\item{join_keys}{(\code{JoinKeys}) nested list of keys used for joining} +\item{join_keys}{(\code{join_keys}) nested list of keys used for joining} } \value{ (\code{list}) simplified selectors with aggregated set of filters, diff --git a/man/get_merge_call.Rd b/man/get_merge_call.Rd index adf232b7..c1138832 100644 --- a/man/get_merge_call.Rd +++ b/man/get_merge_call.Rd @@ -18,7 +18,7 @@ output from \code{\link[=data_extract_multiple_srv]{data_extract_multiple_srv()} When using a reactive named list, the names must be identical to the shiny ids of the respective \code{\link[=data_extract_ui]{data_extract_ui()}}.} -\item{join_keys}{(\code{JoinKeys}) nested list of keys used for joining} +\item{join_keys}{(\code{join_keys}) nested list of keys used for joining} \item{dplyr_call_data}{(\code{list}) simplified selectors with aggregated set of filters,} diff --git a/man/get_merge_key_grid.Rd b/man/get_merge_key_grid.Rd index 727b4f25..075ebd33 100644 --- a/man/get_merge_key_grid.Rd +++ b/man/get_merge_key_grid.Rd @@ -12,7 +12,7 @@ output from \code{\link[=data_extract_multiple_srv]{data_extract_multiple_srv()} When using a reactive named list, the names must be identical to the shiny ids of the respective \code{\link[=data_extract_ui]{data_extract_ui()}}.} -\item{join_keys}{(\code{JoinKeys}) nested list of keys used for joining} +\item{join_keys}{(\code{join_keys}) nested list of keys used for joining} } \value{ list of key pairs between all datasets diff --git a/man/get_rename_call.Rd b/man/get_rename_call.Rd index 374c111f..2e0c036f 100644 --- a/man/get_rename_call.Rd +++ b/man/get_rename_call.Rd @@ -19,7 +19,7 @@ When using a reactive named list, the names must be identical to the shiny ids o \item{idx}{optional (\code{integer}) current selector index in all selectors list} -\item{join_keys}{(\code{JoinKeys}) nested list of keys used for joining} +\item{join_keys}{(\code{join_keys}) nested list of keys used for joining} \item{dplyr_call_data}{(\code{list}) simplified selectors with aggregated set of filters, selections, reshapes etc. All necessary data for merging} diff --git a/man/get_reshape_call.Rd b/man/get_reshape_call.Rd index 3c79e8e7..3e447d66 100644 --- a/man/get_reshape_call.Rd +++ b/man/get_reshape_call.Rd @@ -19,7 +19,7 @@ When using a reactive named list, the names must be identical to the shiny ids o \item{idx}{optional (\code{integer}) current selector index in all selectors list} -\item{join_keys}{(\code{JoinKeys}) nested list of keys used for joining} +\item{join_keys}{(\code{join_keys}) nested list of keys used for joining} \item{dplyr_call_data}{(\code{list}) simplified selectors with aggregated set of filters, selections, reshapes etc. All necessary data for merging} diff --git a/man/merge_datasets.Rd b/man/merge_datasets.Rd index a0b6b262..9b9ca000 100644 --- a/man/merge_datasets.Rd +++ b/man/merge_datasets.Rd @@ -22,7 +22,7 @@ When using a reactive named list, the names must be identical to the shiny ids o object containing data as a list of \code{data.frame}. When passing a list of non-reactive \code{data.frame} objects, they are converted to reactive \code{data.frame} objects internally.} -\item{join_keys}{(\code{JoinKeys})\cr +\item{join_keys}{(\code{join_keys})\cr of variables used as join keys for each of the datasets in \code{datasets}. This will be used to extract the \code{keys} of every dataset.} diff --git a/man/merge_expression_module.Rd b/man/merge_expression_module.Rd index e90de22a..799e06b2 100644 --- a/man/merge_expression_module.Rd +++ b/man/merge_expression_module.Rd @@ -18,7 +18,7 @@ merge_expression_module( object containing data as a list of \code{data.frame}. When passing a list of non-reactive \code{data.frame} objects, they are converted to reactive \code{data.frame} objects internally.} -\item{join_keys}{(\code{JoinKeys})\cr +\item{join_keys}{(\code{join_keys})\cr of variables used as join keys for each of the datasets in \code{datasets}. This will be used to extract the \code{keys} of every dataset.} diff --git a/man/merge_expression_srv.Rd b/man/merge_expression_srv.Rd index 25714370..7b7c10be 100644 --- a/man/merge_expression_srv.Rd +++ b/man/merge_expression_srv.Rd @@ -26,7 +26,7 @@ When using a reactive named list, the names must be identical to the shiny ids o object containing data as a list of \code{data.frame}. When passing a list of non-reactive \code{data.frame} objects, they are converted to reactive \code{data.frame} objects internally.} -\item{join_keys}{(\code{JoinKeys})\cr +\item{join_keys}{(\code{join_keys})\cr of variables used as join keys for each of the datasets in \code{datasets}. This will be used to extract the \code{keys} of every dataset.} diff --git a/man/validate_keys_sufficient.Rd b/man/validate_keys_sufficient.Rd index 6e2e715d..4709e08b 100644 --- a/man/validate_keys_sufficient.Rd +++ b/man/validate_keys_sufficient.Rd @@ -7,7 +7,7 @@ validate_keys_sufficient(join_keys, merged_selector_list) } \arguments{ -\item{join_keys}{(\code{JoinKeys}) the provided join keys} +\item{join_keys}{(\code{join_keys}) the provided join keys} \item{merged_selector_list}{(\code{list}) the specification of datasets' slices to merge} } diff --git a/tests/testthat/test-data_extract_multiple_srv.R b/tests/testthat/test-data_extract_multiple_srv.R index b69c6ba2..a8bfe691 100644 --- a/tests/testthat/test-data_extract_multiple_srv.R +++ b/tests/testthat/test-data_extract_multiple_srv.R @@ -154,7 +154,7 @@ testthat::test_that( ) testthat::test_that( - desc = "data_extract_multiple_srv accepts throws error when join_keys argument is not JoinKeys object", + desc = "data_extract_multiple_srv accepts throws error when join_keys argument is not join_keys object", code = { shiny::withReactiveDomain( domain = shiny::MockShinySession$new(), diff --git a/tests/testthat/test-data_extract_srv.R b/tests/testthat/test-data_extract_srv.R index 167c855e..cb2336cb 100644 --- a/tests/testthat/test-data_extract_srv.R +++ b/tests/testthat/test-data_extract_srv.R @@ -110,7 +110,7 @@ testthat::test_that( ) testthat::test_that( - desc = "data_extract_srv accepts throws error when join_keys argument is not a JoinKeys object", + desc = "data_extract_srv accepts throws error when join_keys argument is not a join_keys object", code = { shiny::withReactiveDomain( domain = shiny::MockShinySession$new(), diff --git a/tests/testthat/test-merge_expression_srv.R b/tests/testthat/test-merge_expression_srv.R index f6e03da7..d7e7abdb 100644 --- a/tests/testthat/test-merge_expression_srv.R +++ b/tests/testthat/test-merge_expression_srv.R @@ -242,14 +242,14 @@ testthat::test_that("merge_expression_srv throws error if datasets is not a name ) }) -testthat::test_that("merge_expression_srv throws error if join_keys is not a JoinKeys object", { +testthat::test_that("merge_expression_srv throws error if join_keys is not a join_keys object", { testthat::expect_error( shiny::testServer( merge_expression_srv, args = list(selector_list = selector_list, datasets = data_list, join_keys = list("USUBJID")), expr = NULL ), - "class 'JoinKeys', but has class 'list'" + "class 'join_keys', but has class 'list'" ) }) From 9bbaa5f4485e16be5a3305316f73feab00ce587d Mon Sep 17 00:00:00 2001 From: vedhav Date: Thu, 16 Nov 2023 18:34:59 +0530 Subject: [PATCH 09/15] chore: fix lint error --- R/get_dplyr_call.R | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/R/get_dplyr_call.R b/R/get_dplyr_call.R index e0724100..e74a37b9 100644 --- a/R/get_dplyr_call.R +++ b/R/get_dplyr_call.R @@ -291,9 +291,11 @@ get_filter_call <- function(filter, dataname = NULL, datasets = NULL) { return(NULL) } - stopifnot((!is.null(dataname) && is.null(datasets)) || - (is.null(dataname) && is.null(datasets)) || - (!is.null(datasets) && isTRUE(dataname %in% names(datasets)))) + stopifnot( + (!is.null(dataname) && is.null(datasets)) || + (is.null(dataname) && is.null(datasets)) || + (!is.null(datasets) && isTRUE(dataname %in% names(datasets))) + ) get_filter_call_internal <- function(filter, dataname, datasets) { if (rlang::is_empty(filter$selected)) { From a6c82d4f432377d226fbb0ad0df89420ebcedd43 Mon Sep 17 00:00:00 2001 From: Vedha Viyash Date: Thu, 16 Nov 2023 18:50:05 +0530 Subject: [PATCH 10/15] fix: fix broken test --- tests/testthat/test-data_extract_multiple_srv.R | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/testthat/test-data_extract_multiple_srv.R b/tests/testthat/test-data_extract_multiple_srv.R index a8bfe691..494f02d7 100644 --- a/tests/testthat/test-data_extract_multiple_srv.R +++ b/tests/testthat/test-data_extract_multiple_srv.R @@ -100,9 +100,9 @@ testthat::test_that("data_extract_multiple_srv accepts datasets as FilteredData mixed_data_list <- list(IRIS = reactive(iris), IRIS2 = iris) mixed_join_keys_list <- teal.data::join_keys( - teal.data::join_key("IRIS", "IRIS", character(0)), - teal.data::join_key("IRIS2", "IRIS2", character(0)), - teal.data::join_key("IRIS", "IRIS2", character(0)) + teal.data::join_key("IRIS", "IRIS", "id"), + teal.data::join_key("IRIS2", "IRIS2", "id"), + teal.data::join_key("IRIS", "IRIS2", "id") ) shiny::withReactiveDomain( From 143e218baa19cbe9c63196a48672b4fae69ea7fc Mon Sep 17 00:00:00 2001 From: go_gonzo Date: Fri, 17 Nov 2023 06:37:16 +0100 Subject: [PATCH 11/15] replace [[ i ]][[ j ]] to [i, j] --- R/data_extract_module.R | 4 ++-- R/get_merge_call.R | 2 +- R/merge_datasets.R | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/R/data_extract_module.R b/R/data_extract_module.R index 4e924c48..33434648 100644 --- a/R/data_extract_module.R +++ b/R/data_extract_module.R @@ -456,8 +456,8 @@ data_extract_srv.list <- function(id, datasets, data_extract_spec, join_keys = N ) # get keys out of join_keys - if (!is.null(join_keys)) { - keys <- sapply(names(datasets), simplify = FALSE, function(x) join_keys[[x]][[x]]) + if (length(join_keys)) { + keys <- sapply(names(datasets), simplify = FALSE, function(x) join_keys[x, x]) } else { keys <- sapply(names(datasets), simplify = FALSE, function(x) character(0)) } diff --git a/R/get_merge_call.R b/R/get_merge_call.R index dd7ad272..4d669d20 100644 --- a/R/get_merge_call.R +++ b/R/get_merge_call.R @@ -130,7 +130,7 @@ get_merge_key_grid <- function(selector_list, join_keys = teal.data::join_keys() get_merge_key_pair( selector_from, selector_to, - join_keys[[selector_from$dataname]][[selector_to$dataname]] + join_keys[selector_from$dataname, selector_to$dataname] ) } ) diff --git a/R/merge_datasets.R b/R/merge_datasets.R index 98fcc79c..93fa1da7 100644 --- a/R/merge_datasets.R +++ b/R/merge_datasets.R @@ -280,7 +280,7 @@ are_needed_keys_provided <- function(join_keys, merged_selector_list) { } do_join_keys_exist <- function(dataset_name1, dataset_name2, join_keys) { - length(join_keys[[dataset_name1]][[dataset_name2]] > 0) + length(join_keys[dataset_name1, dataset_name2] > 0) } datasets_names <- vapply(merged_selector_list, function(slice) slice[["dataname"]], FUN.VALUE = character(1)) From ff818b4260c9fb05a08add069531efd0e49316f9 Mon Sep 17 00:00:00 2001 From: go_gonzo Date: Fri, 17 Nov 2023 06:57:52 +0100 Subject: [PATCH 12/15] reload From 6c5e8b9021c0b937042ded12e2089ad2d15126a4 Mon Sep 17 00:00:00 2001 From: vedhav Date: Fri, 17 Nov 2023 15:40:23 +0530 Subject: [PATCH 13/15] chore: using the proper join_keys subset --- tests/testthat/test-merge_expression_srv.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/testthat/test-merge_expression_srv.R b/tests/testthat/test-merge_expression_srv.R index d7e7abdb..ceff634f 100644 --- a/tests/testthat/test-merge_expression_srv.R +++ b/tests/testthat/test-merge_expression_srv.R @@ -20,7 +20,7 @@ adsl_data_extract_srv_output <- dataname = "ADSL", filters = NULL, select = "AGE", - keys = join_keys[["ADSL"]][["ADSL"]], + keys = join_keys["ADSL", "ADSL"], reshape = FALSE, internal_id = "adsl_extract" ) @@ -30,7 +30,7 @@ adlb_data_extract_srv_output <- dataname = "ADLB", filters = NULL, select = c("AVAL", "CHG"), - keys = join_keys[["ADLB"]][["ADLB"]], + keys = join_keys["ADLB", "ADLB"], reshape = FALSE, internal_id = "adlb_extract" ) From 9c504e79ff67793dd5efaa62e06845e242b1068f Mon Sep 17 00:00:00 2001 From: vedhav Date: Fri, 17 Nov 2023 18:09:30 +0530 Subject: [PATCH 14/15] chore: replace `runApp` with `shinyApp` --- R/data_extract_module.R | 6 +++--- R/merge_expression_module.R | 4 ++-- R/utils.R | 2 +- man/compose_and_enable_validators.Rd | 2 +- man/data_extract_multiple_srv.Rd | 2 +- man/data_extract_srv.Rd | 4 ++-- man/merge_expression_module.Rd | 2 +- man/merge_expression_srv.Rd | 2 +- 8 files changed, 12 insertions(+), 12 deletions(-) diff --git a/R/data_extract_module.R b/R/data_extract_module.R index 33434648..87f2646a 100644 --- a/R/data_extract_module.R +++ b/R/data_extract_module.R @@ -346,7 +346,7 @@ check_data_extract_spec_react <- function(datasets, data_extract) { #' } #' ) #' if (interactive()) { -#' runApp(app) +#' shinyApp(app$ui, app$server) #' } #' #' # Using FilteredData - Note this method will be deprecated @@ -381,7 +381,7 @@ check_data_extract_spec_react <- function(datasets, data_extract) { #' } #' ) #' if (interactive()) { -#' runApp(app) +#' shinyApp(app$ui, app$server) #' } data_extract_srv <- function(id, datasets, data_extract_spec, ...) { checkmate::assert_multi_class(datasets, c("FilteredData", "list")) @@ -651,7 +651,7 @@ data_extract_srv.list <- function(id, datasets, data_extract_spec, join_keys = N #' } #' ) #' if (interactive()) { -#' runApp(app) +#' shinyApp(app$ui, app$server) #' } data_extract_multiple_srv <- function(data_extract, datasets, ...) { checkmate::assert_list(data_extract, names = "named") diff --git a/R/merge_expression_module.R b/R/merge_expression_module.R index 7dd518f0..daf5f295 100644 --- a/R/merge_expression_module.R +++ b/R/merge_expression_module.R @@ -130,7 +130,7 @@ #' } #' ) #' \dontrun{ -#' runApp(app) +#' shinyApp(app$ui, app$server) #' } #' @export merge_expression_module <- function(datasets, @@ -303,7 +303,7 @@ merge_expression_module <- function(datasets, #' } #' ) #' \dontrun{ -#' runApp(app) +#' shinyApp(app$ui, app$server) #' } merge_expression_srv <- function(id = "merge_id", selector_list, diff --git a/R/utils.R b/R/utils.R index 13d26fc2..67495a91 100644 --- a/R/utils.R +++ b/R/utils.R @@ -139,7 +139,7 @@ extract_choices_labels <- function(choices, values = NULL) { #' } #' ) #' if (interactive()) { -#' runApp(app) +#' shinyApp(app$ui, app$server) #' } compose_and_enable_validators <- function(iv, selector_list, validator_names = NULL) { if (is.null(validator_names)) { diff --git a/man/compose_and_enable_validators.Rd b/man/compose_and_enable_validators.Rd index ac90e657..cddce526 100644 --- a/man/compose_and_enable_validators.Rd +++ b/man/compose_and_enable_validators.Rd @@ -109,6 +109,6 @@ app <- shinyApp( } ) if (interactive()) { - runApp(app) + shinyApp(app$ui, app$server) } } diff --git a/man/data_extract_multiple_srv.Rd b/man/data_extract_multiple_srv.Rd index fe52d972..0e7e679a 100644 --- a/man/data_extract_multiple_srv.Rd +++ b/man/data_extract_multiple_srv.Rd @@ -152,6 +152,6 @@ app <- shinyApp( } ) if (interactive()) { - runApp(app) + shinyApp(app$ui, app$server) } } diff --git a/man/data_extract_srv.Rd b/man/data_extract_srv.Rd index 376d1264..75d228ee 100644 --- a/man/data_extract_srv.Rd +++ b/man/data_extract_srv.Rd @@ -144,7 +144,7 @@ app <- shinyApp( } ) if (interactive()) { - runApp(app) + shinyApp(app$ui, app$server) } # Using FilteredData - Note this method will be deprecated @@ -179,7 +179,7 @@ app <- shinyApp( } ) if (interactive()) { - runApp(app) + shinyApp(app$ui, app$server) } } \references{ diff --git a/man/merge_expression_module.Rd b/man/merge_expression_module.Rd index 799e06b2..b452b401 100644 --- a/man/merge_expression_module.Rd +++ b/man/merge_expression_module.Rd @@ -152,7 +152,7 @@ app <- shinyApp( } ) \dontrun{ -runApp(app) +shinyApp(app$ui, app$server) } } \seealso{ diff --git a/man/merge_expression_srv.Rd b/man/merge_expression_srv.Rd index 7b7c10be..27dce5f3 100644 --- a/man/merge_expression_srv.Rd +++ b/man/merge_expression_srv.Rd @@ -161,7 +161,7 @@ app <- shinyApp( } ) \dontrun{ -runApp(app) +shinyApp(app$ui, app$server) } } \seealso{ From a55f412a04a530c1440496b97143a896f93f1591 Mon Sep 17 00:00:00 2001 From: vedhav Date: Mon, 20 Nov 2023 16:00:52 +0530 Subject: [PATCH 15/15] chore: vbump --- DESCRIPTION | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 7c992447..d4bd1788 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -32,9 +32,9 @@ Imports: shinyjs, shinyvalidate, stats, - teal.data (>= 0.3.0), + teal.data (>= 0.3.0.9010), teal.logger (>= 0.1.1), - teal.slice (>= 0.4.0), + teal.slice (>= 0.4.0.9023), teal.widgets (>= 0.4.0), tidyr (>= 0.8.3), tidyselect