1299 do not show datanames error if there are transformers passed to …

…modules (#1319) Fixes the top part of #1299 The current behavior raised a warning that datanames are not sufficient based on comparison of `data@datanames` and `modules$datanames`. This now have a new condition - warning is only displayed when the above is satisfied & there are no `transformers`. <img width="697" alt="image" src="https://github.com/user-attachments/assets/055f4982-07c3-4157-823f-af448443f537"> # Code for testing <details><summary>Tested with</summary> ```r pkgload::load_all('teal.data') pkgload::load_all('teal.slice') pkgload::load_all('teal') my_transformers <- list( teal_transform_module( label = "Custom transform for iris", ui = function(id) { ns <- NS(id) tags$div( numericInput(ns("n_rows"), "Subset n rows", value = 6, min = 1, max = 150, step = 1) ) }, server = function(id, data) { moduleServer(id, function(input, output, session) { reactive({ within(data(), { iris <- head(iris, num_rows) }, num_rows = input$n_rows ) }) }) } ) ) app1 <- init( data = teal_data(iris = iris), modules = example_module(datanames = c("iris", "mtcars")) ) shinyApp(app1$ui, app1$server) app2 <- init( data = teal_data(iris = iris), modules = example_module(datanames = c("iris", "mtcars"), transformers = my_transformers) ) shinyApp(app2$ui, app2$server) ``` </details> # No transformers - warning <img width="658" alt="no_transformers" src="https://github.com/user-attachments/assets/cc90a3ae-f73c-4f72-ab11-6eb9efd2acf9"> # Transformers exist - no warning <img width="662" alt="transformers" src="https://github.com/user-attachments/assets/d560193b-72ff-4b87-9a9e-0d2ccf90a5e9"> # Results of local tests <details><summary>Results of local tests</summary> ```r > devtools::test() ℹ Testing teal [INFO] 2024-08-14 11:40:00.4037 pid:11652 token:[] teal You are using teal version 0.15.2.9052 ✔ | F W S OK | Context ✔ | 10 | init [1.1s] ✔ | 6 129 | module_teal [57.3s] ✔ | 95 | modules ✔ | 7 | rcode_utils ✔ | 8 | report_previewer_module ✔ | 6 0 | shinytest2-data_summary ✔ | 3 0 | shinytest2-filter_panel ✔ | 3 0 | shinytest2-init ✔ | 5 0 | shinytest2-landing_popup ✔ | 4 0 | shinytest2-module_bookmark_manager ✔ | 4 0 | shinytest2-modules ✔ | 5 0 | shinytest2-reporter ✔ | 1 0 | shinytest2-show-rcode ✔ | 6 0 | shinytest2-teal_data_module ✔ | 2 0 | shinytest2-teal_slices ✔ | 1 0 | shinytest2-utils ✔ | 2 0 | shinytest2-wunder_bar ✔ | 16 | teal_data_module-eval_code ✔ | 4 | teal_data_module ✔ | 25 | teal_reporter ✔ | 15 | teal_slices-store ✔ | 18 | teal_slices ✔ | 36 | utils [9.7s] ✔ | 17 | validate_has_data ✔ | 36 | validate_inputs ══ Results ════════════════════════════════════════════════════════════════════════════ Duration: 74.8 s ── Skipped tests (48) ───────────────────────────────────────────────────────────────── • need a fix in a .slicesGlobal (1): test-module_teal.R:1177:11 • testing depth 3 is below current testing specification 5 (42): test-shinytest2-data_summary.R:2:3, test-shinytest2-data_summary.R:21:3, test-shinytest2-data_summary.R:40:3, test-shinytest2-data_summary.R:62:3, test-shinytest2-data_summary.R:92:5, test-shinytest2-data_summary.R:139:3, test-shinytest2-filter_panel.R:5:3, test-shinytest2-filter_panel.R:32:3, test-shinytest2-filter_panel.R:71:3, test-shinytest2-init.R:5:3, test-shinytest2-init.R:15:3, test-shinytest2-init.R:70:3, test-shinytest2-landing_popup.R:5:3, test-shinytest2-landing_popup.R:25:3, test-shinytest2-landing_popup.R:43:3, test-shinytest2-landing_popup.R:67:5, test-shinytest2-landing_popup.R:146:3, test-shinytest2-module_bookmark_manager.R:5:3, test-shinytest2-module_bookmark_manager.R:19:3, test-shinytest2-module_bookmark_manager.R:33:3, test-shinytest2-module_bookmark_manager.R:44:3, test-shinytest2-modules.R:5:3, test-shinytest2-modules.R:44:3, test-shinytest2-modules.R:60:3, test-shinytest2-modules.R:77:3, test-shinytest2-reporter.R:5:3, test-shinytest2-reporter.R:25:3, test-shinytest2-reporter.R:48:3, test-shinytest2-reporter.R:87:3, test-shinytest2-reporter.R:103:3, test-shinytest2-show-rcode.R:5:3, test-shinytest2-teal_data_module.R:5:3, test-shinytest2-teal_data_module.R:42:3, test-shinytest2-teal_data_module.R:78:3, test-shinytest2-teal_data_module.R:130:3, test-shinytest2-teal_data_module.R:175:3, test-shinytest2-teal_data_module.R:215:3, test-shinytest2-teal_slices.R:5:3, test-shinytest2-teal_slices.R:48:3, test-shinytest2-utils.R:5:3, test-shinytest2-wunder_bar.R:5:3, test-shinytest2-wunder_bar.R:24:3 • todo (5): test-module_teal.R:1442:7, test-module_teal.R:1449:5, test-module_teal.R:1452:5, test-module_teal.R:1710:5, test-module_teal.R:1768:5 [ FAIL 0 | WARN 0 | SKIP 48 | PASS 416 ] ``` </details> --------- Signed-off-by: Marcin <[email protected]> Co-authored-by: 27856297+dependabot-preview[bot]@users.noreply.github.com <27856297+dependabot-preview[bot]@users.noreply.github.com> Co-authored-by: Dawid Kałędkowski <[email protected]> Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
insightsengineering · Aug 28, 2024 · 96db2db · 96db2db
1 parent 2e40342
commit 96db2db
Show file tree

Hide file tree

Showing 4 changed files with 71 additions and 12 deletions.
diff --git a/R/init.R b/R/init.R
@@ -219,8 +219,8 @@ init <- function(data,
     }
 
     is_modules_ok <- check_modules_datanames(modules, .teal_data_datanames(data))
-    if (!isTRUE(is_modules_ok)) {
-      lapply(is_modules_ok$string, logger::log_warn)
+    if (!isTRUE(is_modules_ok) && length(unlist(extract_transformers(modules))) == 0) {
+      lapply(is_modules_ok$string, warning, call. = FALSE)
     }
 
     is_filter_ok <- check_filter_datanames(filter, .teal_data_datanames(data))

diff --git a/R/teal_data_module.R b/R/teal_data_module.R
@@ -152,3 +152,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 `teal_transform_module` nested in 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)
+  }
+}
diff --git a/man/extract_transformers.Rd b/man/extract_transformers.Rd
diff --git a/tests/testthat/test-init.R b/tests/testthat/test-init.R
@@ -56,17 +56,44 @@ 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::local_mocked_bindings(log_warn = warning, .package = "logger")
+testthat::test_that(
+  "init throws warning when datanames in modules incompatible w/ datanames in data and there is no transformers",
+  {
+    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",
+  {
+    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(