Add ability to set datanames

[llrs-roche]

Pull Request

Fixes #821

This PR add a parameter (or uses existing ones) to use on teal::module(datanames) to select which datasets are shown in each module.

tm_file_viewer() doesn't have and I think shouldn't have.

Acceptance criteria:

tm_missing_data example

# general example data

library("devtools")
load_all("../teal.modules.general")
load_all("../teal.code")
load_all("../teal.transform")
data <- teal_data()
data <- within(data, {
  require(nestcolor)
  
  add_nas <- function(x) {
    x[sample(seq_along(x), floor(length(x) * runif(1, .05, .17)))] <- NA
    x
  }
  
  iris <- iris
  .iris <- iris
  iris2 <- iris
  mtcars <- mtcars
  
  iris[] <- lapply(iris, add_nas)
  .iris[] <- lapply(.iris, add_nas)
  iris2[] <- lapply(iris2, add_nas)
  mtcars[] <- lapply(mtcars, add_nas)
  mtcars[["cyl"]] <- as.factor(mtcars[["cyl"]])
  mtcars[["gear"]] <- as.factor(mtcars[["gear"]])
})

app <- init(
  data = data,
  modules = modules(
    tm_missing_data(datasets_selected = c("iris2", "iris"))
  )
)
if (interactive()) {
  shinyApp(app$ui, app$server)
}

tm_variable_browser example

library("devtools")
load_all("../teal.modules.general")
load_all("../teal.code")
load_all("../teal.transform")
# general data example
data <- teal_data()
data <- within(data, {
  iris <- iris
  mtcars <- mtcars
  women <- women
  faithful <- faithful
  CO2 <- CO2
})

app <- init(
  data = data,
  modules = modules(
    tm_variable_browser(
      label = "Variable browser",
      datasets_selected = c("iris", "mtcars")
    )
  )
)
if (interactive()) {
  shinyApp(app$ui, app$server)
}

tm_front_page (additional modifications to enable granular selection): Example

library("devtools")
load_all("../teal.modules.general")
load_all("../teal.code")
load_all("../teal.transform")
data <- teal_data()
data <- within(data, {
  require(nestcolor)
  ADSL <- teal.data::rADSL
  ADSL2 <- ADSL
  attr(ADSL, "metadata") <- list("Author" = "NEST team", "data_source" = "synthetic data")
})
join_keys(data) <- default_cdisc_join_keys[names(data)]

table_1 <- data.frame(Info = c("A", "B"), Text = c("A", "B"))
table_2 <- data.frame(`Column 1` = c("C", "D"), `Column 2` = c(5.5, 6.6), `Column 3` = c("A", "B"))
table_3 <- data.frame(Info = c("E", "F"), Text = c("G", "H"))

table_input <- list(
  "Table 1" = table_1,
  "Table 2" = table_2,
  "Table 3" = table_3
)

app <- init(
  data = data,
  modules = modules(
    tm_front_page(
      header_text = c(
        "Important information" = "It can go here.",
        "Other information" = "Can go here."
      ),
      tables = table_input,
      additional_tags = HTML("Additional HTML or shiny tags go here <br>"),
      footnotes = c("X" = "is the first footnote", "Y is the second footnote"),
      show_metadata = TRUE,
      datasets_selected = "ADSL2"
    )
  ),
  header = tags$h1("Sample Application"),
  footer = tags$p("Application footer"),
)

if (interactive()) {
  shinyApp(app$ui, app$server)
}

I checked via searching datanames = that there are now at least 15 cases in module() corresponding to the 15 modules exported.

There is one that is not configurable (datanames = NULL on the picture above), tm_file_viewer. In my opinion it doesn't matter the datasets available on the module to visualize files. But if needed we can add a dataset_selected argument.

Note that I added the dataset_selected argument to two modules on the best place I thought, if a package/user is using positional arguments it might break (and also teal.gallery).

Note that it doesn't show on shinylive (on Rstudio, preview, not sure if once fully installed) and also that it doesn't show the url (this could be an issue with my machine):

[github-actions]

Unit Tests Summary

  1 files   22 suites   13m 1s ⏱️
145 tests 107 ✅ 38 💤 0 ❌
476 runs  438 ✅ 38 💤 0 ❌

Results for commit 0deb55e.

♻️ This comment has been updated with latest results.

[github-actions]

Unit Test Performance Difference

Test Suite	$Status$	Time on main	$±Time$	$±Tests$	$±Skipped$	$±Failures$	$±Errors$
shinytest2-tm_outliers	💔	$110.02$	$+1.60$	$0$	$0$	$0$	$0$

Results for commit 08710df

♻️ This comment has been updated with latest results.

[llrs-roche]

Thanks, I'll fix the style checks for the documentation too.

[gogonzo]

Please remove the tests for deprecated messages. Warning is thrown by the tm_ constructor. Testing it through the app driver doesn't make any sense as the message is not thrown in shiny runtime.

[llrs-roche]

When I'm re-running the cases above, I'm still facing an issue with the tm_front_page, it seems to come from show_metadata = deprecated(), I see on the terminal:

[WARN] 2025-01-14 15:05:22.8165 pid:26548 token:[] teal.modules.general Error in <Anonymous>: argument is missing, with no default
  87: <Anonymous> [C:/Code/Rprojects/teal.modules.general/R/tm_front_page.R#116]
  81: ui_teal_module.teal_module
  73: FUN
  72: lapply
  68: ui_teal_module.teal_modules
  59: ui_teal
  58: ui
   1: runApp

I'm exploring why this is different from other modules

[gogonzo]

When I'm re-running the cases above, I'm still facing an issue with the tm_front_page, it seems to come from show_metadata = deprecated(), I see on the terminal:

[WARN] 2025-01-14 15:05:22.8165 pid:26548 token:[] teal.modules.general Error in <Anonymous>: argument is missing, with no default
  87: <Anonymous> [C:/Code/Rprojects/teal.modules.general/R/tm_front_page.R#116]
  81: ui_teal_module.teal_module
  73: FUN
  72: lapply
  68: ui_teal_module.teal_modules
  59: ui_teal
  58: ui
   1: runApp

I'm exploring why this is different from other modules

You've stepped into funny bug. lifecycle::deprecated is of language type and it is too-early-evaluated in teal. Run the example with branch from this PR.

[llrs-roche]

I approved the PR mentioned. If there is anything else to improve let me know

[gogonzo]

Thanks 👍

[llrs-roche]

I can reproduce the test failing locally now (before they were sporadic/ not consistent) running through all this branch's commits to tests which one broke them (git bisect run testthat_cli.R where testthat_cli.R just has devtools::load_all(".");devtools::test()) but not sure if this is a similar ghost action as in insightsengineering/teal.modules.clinical#1309.

[llrs-roche]

Bisect finished:

4cab9d5cdf6f19d2fd1d33f7712337dbfc1b2f9a was both good and bad
error: bisect run failed: 'git bisect good' exited with error code -1

But results don't seem deterministic, as the commit can be both good and bad to run the tests.
This is with locally installed versions from github, next step will be to install nest packages from pharmaverse.r-universe.org (if firewall allows it).

[llrs-roche]

I can now reproduce the failing checks locally (both on Rstudio R terminal and on the check window pane).

Running the tests manually, shows additional information. From this failing test, whose error is reported as:

Error (test-shinytest2-tm_misssing_data.R:154:3): e2e - tm_missing_data: Validate 'By Variable Levels' table values
Error in `UseMethod("read_xml")`: no applicable method for 'read_xml' applied to an object of class "NULL"

I run the code manually and see more details, the app app_driver_tm_missing_data() generates some warnings:

Details

load_all("../teal.modules.general")
data <- within(simple_teal_data(), {
  set.seed(123)
  
  add_nas <- function(x) {
    x[sample(seq_along(x), floor(length(x) * runif(1, .05, .17)))] <- NA
    x
  }
  
  iris[] <- lapply(iris, add_nas)
  mtcars[] <- lapply(mtcars, add_nas)
  mtcars[["cyl"]] <- as.factor(mtcars[["cyl"]])
  mtcars[["gear"]] <- as.factor(mtcars[["gear"]])
})
app <- teal::init(
  data = data,
  modules = tm_missing_data(
    label = "Missing data",
    plot_height = c(600, 400, 5000),
    plot_width = NULL,
    parent_dataname = "",
    ggtheme = "gray",
    ggplot2_args = list(
      "Combinations Hist" = teal.widgets::ggplot2_args(
        labs = list(subtitle = "Plot produced by Missing Data Module", caption = NULL)
      ),
      "Combinations Main" = teal.widgets::ggplot2_args(labs = list(title = NULL))
    ),
    pre_output = NULL,
    post_output = NULL
  )
)

## Initializing reporter_previewer_module
## Warning messages:
## 1: In rlang::hash(list(data = data, modules = modules)) :
##   'package:jsonlite' may not be available when loading
## 2: Dataset `` is missing for module 'Missing data'. Datasets available in data: `add_nas`, `iris` and `mtcars`. 

^{Created on 2025-01-16 with reprex v2.1.1}

I fixed those and pushed to see if anything else come up.

[llrs-roche]

I thought that the remaining failing tests could be due to the code generated by each branch. But running them and comparing the (html) code generated, didn't help to identify the issue.

diff tmg_source_code_main.html tmg_source_code_branch.html

It might be either some hidden component, but I'm not familiar with this part of shiny/testing/web development

[llrs-roche]

@gogonzo All checks pass now. If there is no more comments I'll merge the PR this afternoon. Thanks for helping with the ordering issue.

[vedhav]

@llrs-roche I was testing some of these modules which are used in teal.gallery and found some odd behavior.

In general, when the datanames is "all" we expect all the datanames to be passed into the module, which is not the case now. Right now we get the module output with no access to any datasets that are passed. This is true for tm_data_table, tm_variable_browser, and tm_missing_data.
Additionally, I think the default value for the datanames should be NULL for the tm_front_page module so the filter panel will be hidden, this is generally the expectation for a front page.

Here's a quick example. You can also try running the module examples to see that they also fail.

devtools::load_all("../teal.modules.general")
app <- init(
  data = teal_data(iris = iris, mtcars = mtcars),
  modules = modules(
    tm_data_table(),
    tm_variable_browser(),
    tm_missing_data()
  )
)

shinyApp(app$ui, app$server)

[llrs-roche]

@vedhav thanks for the message. Yes, I noticed and there is a different issue opened to fix those three modules: #1452 . I'm working to fix those three modules and check on the datanames "all" and NULL cases. Apologies for he trouble it caused. I'll keep in mind your suggestion about tm_front_page.