From 352c1a76177ac413f0255dd31126d717cab65901 Mon Sep 17 00:00:00 2001 From: m7pr Date: Wed, 14 Aug 2024 11:42:01 +0200 Subject: [PATCH 01/12] do not show datanames error if there are some transformers passed to modules --- R/init.R | 2 +- R/module_teal_data.R | 2 +- R/teal_data_module.R | 14 ++++++++++++++ 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/R/init.R b/R/init.R index 092c263dd5..e0b5f49bf6 100644 --- a/R/init.R +++ b/R/init.R @@ -224,7 +224,7 @@ init <- function(data, } is_modules_ok <- check_modules_datanames(modules, .teal_data_datanames(data)) - if (!isTRUE(is_modules_ok)) { + if (!isTRUE(is_modules_ok) && length(unlist(extract_transformers(modules))) == 0) { lapply(is_modules_ok$string, logger::log_warn) } diff --git a/R/module_teal_data.R b/R/module_teal_data.R index f2bbaf4145..25ff581520 100644 --- a/R/module_teal_data.R +++ b/R/module_teal_data.R @@ -162,7 +162,7 @@ srv_validate_reactive_teal_data <- function(id, # nolint: object_length output$shiny_warnings <- renderUI({ if (inherits(data_out_rv(), "teal_data")) { is_modules_ok <- check_modules_datanames(modules = modules, datanames = .teal_data_ls(data_validated())) - if (!isTRUE(is_modules_ok)) { + if (!isTRUE(is_modules_ok) && length(unlist(extract_transformers(modules))) == 0) { tags$div( is_modules_ok$html( # Show modules prefix on message only in teal_data_module tab diff --git a/R/teal_data_module.R b/R/teal_data_module.R index eddf1fa277..c8b29f7231 100644 --- a/R/teal_data_module.R +++ b/R/teal_data_module.R @@ -129,3 +129,17 @@ teal_transform_module <- function(ui, server, label = "transform module") { class = c("teal_transform_module", "teal_data_module") ) } + + +#' Extract all `transformers` from `modules`. +#' +#' @param modules `teal_modules` or `teal_module` +#' @return A list of `transformers` nested the same way as input `modules`. +#' @keywords internal +extract_transformers <- function(modules) { + if (inherits(modules, "teal_module")) { + modules$transformers + } else if (inherits(modules, "teal_modules")) { + lapply(modules$children, extract_transformers) + } +} From 402c6755c050506641840e1cf3eabd2e4bc62a25 Mon Sep 17 00:00:00 2001 From: "27856297+dependabot-preview[bot]@users.noreply.github.com" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Wed, 14 Aug 2024 09:51:59 +0000 Subject: [PATCH 02/12] [skip roxygen] [skip vbump] Roxygen Man Pages Auto Update --- man/extract_transformers.Rd | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 man/extract_transformers.Rd diff --git a/man/extract_transformers.Rd b/man/extract_transformers.Rd new file mode 100644 index 0000000000..ffdfc085d0 --- /dev/null +++ b/man/extract_transformers.Rd @@ -0,0 +1,18 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/teal_data_module.R +\name{extract_transformers} +\alias{extract_transformers} +\title{Extract all \code{transformers} from \code{modules}.} +\usage{ +extract_transformers(modules) +} +\arguments{ +\item{modules}{\code{teal_modules} or \code{teal_module}} +} +\value{ +A list of \code{transformers} nested the same way as input \code{modules}. +} +\description{ +Extract all \code{transformers} from \code{modules}. +} +\keyword{internal} From d701f537d57dd2ddce0c1bd0507ef135d7e4c024 Mon Sep 17 00:00:00 2001 From: m7pr Date: Wed, 14 Aug 2024 11:53:06 +0200 Subject: [PATCH 03/12] extend extra_datanames check with names from transformers --- R/utils.R | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/R/utils.R b/R/utils.R index 410ad9d1ef..c0d1627353 100644 --- a/R/utils.R +++ b/R/utils.R @@ -133,7 +133,10 @@ check_modules_datanames <- function(modules, datanames) { } ) } else { - extra_datanames <- setdiff(modules$datanames, c("all", datanames)) + extra_datanames <- unique(c( + setdiff(modules$datanames, c("all", datanames)), + setdiff(unlist(lapply(modules$transformers, function(x) x$datanames)), c("all", datanames)) + )) if (length(extra_datanames)) { list( string = build_datanames_error_message( From 543ffd1b4f8e96950b50cc3c48d2168ffe705e89 Mon Sep 17 00:00:00 2001 From: Marcin <133694481+m7pr@users.noreply.github.com> Date: Wed, 14 Aug 2024 12:23:46 +0200 Subject: [PATCH 04/12] Update R/init.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: Marcin <133694481+m7pr@users.noreply.github.com> --- R/init.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/init.R b/R/init.R index e0b5f49bf6..f68a058ed2 100644 --- a/R/init.R +++ b/R/init.R @@ -225,7 +225,7 @@ init <- function(data, is_modules_ok <- check_modules_datanames(modules, .teal_data_datanames(data)) if (!isTRUE(is_modules_ok) && length(unlist(extract_transformers(modules))) == 0) { - lapply(is_modules_ok$string, logger::log_warn) +lapply(is_modules_ok$string, warning, call. = FALSE) } is_filter_ok <- check_filter_datanames(filter, .teal_data_datanames(data)) From e966b73470875462f08fb59ac283bf4d64a38d54 Mon Sep 17 00:00:00 2001 From: Marcin <133694481+m7pr@users.noreply.github.com> Date: Wed, 14 Aug 2024 12:24:11 +0200 Subject: [PATCH 05/12] Update R/teal_data_module.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: Marcin <133694481+m7pr@users.noreply.github.com> --- R/teal_data_module.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/teal_data_module.R b/R/teal_data_module.R index c8b29f7231..50cb14b6f7 100644 --- a/R/teal_data_module.R +++ b/R/teal_data_module.R @@ -134,7 +134,7 @@ teal_transform_module <- function(ui, server, label = "transform module") { #' Extract all `transformers` from `modules`. #' #' @param modules `teal_modules` or `teal_module` -#' @return A list of `transformers` nested the same way as input `modules`. +#' @return A list of `teal_transform_module` nested in the same way as input `modules`. #' @keywords internal extract_transformers <- function(modules) { if (inherits(modules, "teal_module")) { From 959817219df82d71a2b363d0eb381c3b6aeb0111 Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 14 Aug 2024 10:26:13 +0000 Subject: [PATCH 06/12] [skip style] [skip vbump] Restyle files --- R/init.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/init.R b/R/init.R index f68a058ed2..831d48293d 100644 --- a/R/init.R +++ b/R/init.R @@ -225,7 +225,7 @@ init <- function(data, is_modules_ok <- check_modules_datanames(modules, .teal_data_datanames(data)) if (!isTRUE(is_modules_ok) && length(unlist(extract_transformers(modules))) == 0) { -lapply(is_modules_ok$string, warning, call. = FALSE) + lapply(is_modules_ok$string, warning, call. = FALSE) } is_filter_ok <- check_filter_datanames(filter, .teal_data_datanames(data)) From d802a0b799c29efd4668fa4afd7f4b0738fe64d3 Mon Sep 17 00:00:00 2001 From: "27856297+dependabot-preview[bot]@users.noreply.github.com" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Wed, 14 Aug 2024 10:31:06 +0000 Subject: [PATCH 07/12] [skip roxygen] [skip vbump] Roxygen Man Pages Auto Update --- man/extract_transformers.Rd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/man/extract_transformers.Rd b/man/extract_transformers.Rd index ffdfc085d0..9af99abe12 100644 --- a/man/extract_transformers.Rd +++ b/man/extract_transformers.Rd @@ -10,7 +10,7 @@ extract_transformers(modules) \item{modules}{\code{teal_modules} or \code{teal_module}} } \value{ -A list of \code{transformers} nested the same way as input \code{modules}. +A list of \code{teal_transform_module} nested in the same way as input \code{modules}. } \description{ Extract all \code{transformers} from \code{modules}. From 7a1bf0b693512ee54e37be7f51293b32eafa8c15 Mon Sep 17 00:00:00 2001 From: m7pr Date: Wed, 14 Aug 2024 13:30:30 +0200 Subject: [PATCH 08/12] add a test for init for the case with incomplete datanames but with transformers --- tests/testthat/test-init.R | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/tests/testthat/test-init.R b/tests/testthat/test-init.R index 24efa4d119..e491c5e2c5 100644 --- a/tests/testthat/test-init.R +++ b/tests/testthat/test-init.R @@ -56,7 +56,8 @@ testthat::test_that("init throws when an empty `data` is used", { ) }) -testthat::test_that("init throws warning when datanames in modules incompatible w/ datanames in data", { +testthat::test_that( + "init throws warning when datanames in modules incompatible w/ datanames in data and there is no transformers", { testthat::local_mocked_bindings(log_warn = warning, .package = "logger") testthat::expect_warning( @@ -68,6 +69,34 @@ testthat::test_that("init throws warning when datanames in modules incompatible ) }) +testthat::test_that( + "init does not throw warning when datanames in modules incompatible w/ datanames in data and there are transformers", + { + testthat::local_mocked_bindings(log_warn = warning, .package = "logger") + + testthat::expect_no_warning( + init( + data = teal.data::teal_data(mtcars = mtcars), + modules = list( + example_module( + datanames = "iris", + transformers = list( + teal_transform_module( + ui = function(id) NULL, + server = function(id, data) { + moduleServer(id, function(input, output, session) { + NULL + }) + } + ) + ) + ) + ) + ) + ) + } +) + testthat::test_that("init throws when dataname in filter incompatible w/ datanames in data", { testthat::expect_warning( init( From 1951be1ed1523773cf254d649b46aa5a0581c90c Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 14 Aug 2024 11:32:29 +0000 Subject: [PATCH 09/12] [skip style] [skip vbump] Restyle files --- tests/testthat/test-init.R | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/tests/testthat/test-init.R b/tests/testthat/test-init.R index e491c5e2c5..399727929a 100644 --- a/tests/testthat/test-init.R +++ b/tests/testthat/test-init.R @@ -57,17 +57,19 @@ testthat::test_that("init throws when an empty `data` is used", { }) testthat::test_that( - "init throws warning when datanames in modules incompatible w/ datanames in data and there is no transformers", { - testthat::local_mocked_bindings(log_warn = warning, .package = "logger") + "init throws warning when datanames in modules incompatible w/ datanames in data and there is no transformers", + { + testthat::local_mocked_bindings(log_warn = warning, .package = "logger") - testthat::expect_warning( - init( - data = teal.data::teal_data(mtcars = mtcars), - modules = list(example_module(datanames = "iris")) - ), - "Dataset \"iris\" is missing for tab 'example teal module'. Dataset available in data: \"mtcars\"." - ) -}) + testthat::expect_warning( + init( + data = teal.data::teal_data(mtcars = mtcars), + modules = list(example_module(datanames = "iris")) + ), + "Dataset \"iris\" is missing for tab 'example teal module'. Dataset available in data: \"mtcars\"." + ) + } +) testthat::test_that( "init does not throw warning when datanames in modules incompatible w/ datanames in data and there are transformers", From d2a658fa74f2286ec813bd7d1936987aa1cc4832 Mon Sep 17 00:00:00 2001 From: m7pr Date: Wed, 14 Aug 2024 13:39:37 +0200 Subject: [PATCH 10/12] add a test in test-module_teal.R for testing the case of insufficient datanames but provided transformers --- tests/testthat/test-module_teal.R | 34 +++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/testthat/test-module_teal.R b/tests/testthat/test-module_teal.R index b308406a54..669e48ab6a 100644 --- a/tests/testthat/test-module_teal.R +++ b/tests/testthat/test-module_teal.R @@ -520,6 +520,40 @@ testthat::describe("srv_teal teal_modules", { ) }) + testthat::it("does not throw a warning when datanames are insufficient but transformers are provided", { + testthat::skip_if_not_installed("rvest") + shiny::testServer( + app = srv_teal, + args = list( + id = "test", + data = teal_data(mtcars = mtcars), + modules = modules( + module( + "module_1", + server = function(id, data) data, + datanames = c("iris"), + transformers = list( + teal_transform_module( + ui = function(id) NULL, + server = function(id, data) { + moduleServer(id, function(input, output, session) { + reactive({NULL}) + }) + } + ) + ) + ) + ) + ), + expr = { + session$setInputs(`teal_modules-active_tab` = "module_1") + testthat::expect_null( + output[["teal_modules-module_1-validate_datanames-shiny_warnings"]] + ) + } + ) + }) + testthat::it("is called and receives data even if datanames in `teal_data` are not sufficient", { data <- teal_data(iris = iris) teal.data::datanames(data) <- "iris" From 1bc54c2588b7ecf8379e5d8ef2720ac4349392b7 Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 14 Aug 2024 11:42:32 +0000 Subject: [PATCH 11/12] [skip style] [skip vbump] Restyle files --- tests/testthat/test-module_teal.R | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/testthat/test-module_teal.R b/tests/testthat/test-module_teal.R index 669e48ab6a..010acc6b80 100644 --- a/tests/testthat/test-module_teal.R +++ b/tests/testthat/test-module_teal.R @@ -537,7 +537,9 @@ testthat::describe("srv_teal teal_modules", { ui = function(id) NULL, server = function(id, data) { moduleServer(id, function(input, output, session) { - reactive({NULL}) + reactive({ + NULL + }) }) } ) From d6276bb189a3b37c03523c89b27c9bd5695015d8 Mon Sep 17 00:00:00 2001 From: m7pr Date: Wed, 14 Aug 2024 13:44:24 +0200 Subject: [PATCH 12/12] remove testthat::local_mocked_bindings --- tests/testthat/test-init.R | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/testthat/test-init.R b/tests/testthat/test-init.R index 399727929a..d0a022330c 100644 --- a/tests/testthat/test-init.R +++ b/tests/testthat/test-init.R @@ -59,8 +59,6 @@ testthat::test_that("init throws when an empty `data` is used", { testthat::test_that( "init throws warning when datanames in modules incompatible w/ datanames in data and there is no transformers", { - testthat::local_mocked_bindings(log_warn = warning, .package = "logger") - testthat::expect_warning( init( data = teal.data::teal_data(mtcars = mtcars), @@ -74,8 +72,6 @@ testthat::test_that( testthat::test_that( "init does not throw warning when datanames in modules incompatible w/ datanames in data and there are transformers", { - testthat::local_mocked_bindings(log_warn = warning, .package = "logger") - testthat::expect_no_warning( init( data = teal.data::teal_data(mtcars = mtcars),