Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Display output on tm_data_table, tm_variable_browser, tm_missing_data #829

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

llrs-roche
Copy link
Contributor

@llrs-roche llrs-roche commented Jan 22, 2025

Pull Request

Fixes insightsengineering/teal#1452

This PR works on the edge cases of datanames = "all" and NULL for each three modules, in addition I also revised the decision on tm_front_page to show all modules by default, and reverted back to not showing the panels by default, as per previous behavior and @vedhav compelling comment.

Code used to check this (besides the examples)
library("teal")
devtools::load_all("../teal.modules.general")

data <- teal_data()
data <- within(data, {
  library(dplyr)
  IRIS <- iris
  MTCARS <- mtcars
  # ADSL <- teal.data::rADSL
})

datanames <- "all"

app <- init(
  data = data,
  modules = modules(
    example_module(),
    tm_front_page(datanames = datanames),
    tm_data_table(datanames = datanames),
    tm_variable_browser(datanames = datanames),
    tm_missing_data(datanames = datanames)
  )
)

shinyApp(app$ui, app$server)


datanames <- NULL

app <- init(
  data = data,
  modules = modules(
    example_module(),
    tm_front_page(datanames = datanames),
    tm_data_table(datanames = datanames),
    tm_variable_browser(datanames = datanames),
    tm_missing_data(datanames = datanames)
  )
)

shinyApp(app$ui, app$server)
module "all" NULL
tm_data_table() ✅ 1
tm_variable_browser() ✅ 2
tm_missing_data() ✅ 2

1: No right-panel but the table uses all datasets that are data.frames. I was thinking if variables_selected names should be used when datanames is NULL ? So that something like this should only show IRIS dataset and no left panel.

teal.modules.general::tm_data_table(
   datanames = NULL,
   variables_selected = list(IRIS = colnames(data$IRIS)[1:3])),

But this gets complicated if one wants to keep the standard 6 columns of the datasests as assertion fails:

teal.modules.general::tm_data_table(
  datanames = NULL,
  variables_selected = list(IRIS = colnames(data$IRIS)[1:3], MTCARS = NULL)),

Assertion on 'variables_selected' failed: May only contain the following types: {character}, but element 2 has type 'NULL'.

2: No right-panel but all the datasets that are data.frames are shown.


I also tested whether transformators worked with tm_missing_data (the only one of these three modules with a transformator argument) because the datanames documentation implies it will be merged with it: currently it doesn't and I'm not sure how to fix it.

If I create a transformator and use datanames not equal to all by default the new dataname won't be used.
I need to explore better the interaction between teal::module and the transformator.

reprex with transformators
library("teal")

data <- teal_data()
data <- within(data, {
  library(dplyr)
  IRIS <- iris
  MTCARS <- mtcars
  # ADSL <- teal.data::rADSL
})


transf <- list(teal_transform_module(
  label = "Transformator",
  ui = function(id) {
    ns <- NS(id)
    tags$div(
      numericInput(ns("n_rows"), "Number of rows to subset", value = 32, 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
        )
      })
    })
  },
  datanames = "all"
))


datanames <- "IRIS"

app <- init(
  data = data,
  modules = modules(
    example_module(),
    tm_front_page(datanames = datanames),
    tm_data_table(datanames = datanames),
    tm_variable_browser(datanames = datanames),
    tm_missing_data(datanames = datanames, transformators = transf)
  )
)

shinyApp(app$ui, app$server)

@m7pr
Copy link
Contributor

m7pr commented Jan 22, 2025

@llrs-roche about transformators and datanames. Have you tried to unify names?
Looks like teal_data() only contains IRIS and MTCARS, however module uses only IRIS BUT your transformator tries to change iris

image

@m7pr
Copy link
Contributor

m7pr commented Jan 24, 2025

@llrs-roche about this

teal.modules.general::tm_data_table(
  datanames = NULL,
  variables_selected = list(IRIS = colnames(data$IRIS)[1:3], MTCARS = NULL)),

Assertion on 'variables_selected' failed: May only contain the following types: {character}, but element 2 has type 'NULL'.

What you get if

teal.modules.general::tm_data_table(
  datanames = NULL,
  variables_selected = list(IRIS = colnames(data$IRIS)[1:3], MTCARS = character(0)),

@m7pr
Copy link
Contributor

m7pr commented Jan 24, 2025

Or

MTCARS = names(mtcars)

@llrs-roche
Copy link
Contributor Author

If I use

teal.modules.general::tm_data_table(
  datanames = NULL,
  variables_selected = list(IRIS = colnames(data$IRIS)[1:3], MTCARS = character(0)),

I get:

[INFO] 2025-01-24 10:13:21.1288 pid:10196 token:[] teal.modules.general Initializing tm_missing_data
[INFO] 2025-01-24 10:13:21.1341 pid:10196 token:[] teal.modules.general Initializing tm_data_table
[ERROR] 2025-01-24 10:13:21.1409 pid:10196 token:[] teal.modules.general In 'FUN(X[[i]], ...)': Assertion on 'variables_selected[[i]]' failed: Must have length >= 1, but has length 0.
Error in FUN(X[[i]], ...) : 
  Assertion on 'variables_selected[[i]]' failed: Must have length >= 1, but has length 0.
Calls: init ... lapply -> FUN -> <Anonymous> -> makeAssertion -> mstop

But passing MTCARS = colnames(mtcars) does show all the columns on the module

@m7pr
Copy link
Contributor

m7pr commented Jan 24, 2025

When it comes to transformators, it shows the dataset if the new one has a different name than IRIS/iris

# general example data
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
  mtcars <- mtcars
  
  iris[] <- lapply(iris, add_nas)
  mtcars[] <- lapply(mtcars, add_nas)
  mtcars[["cyl"]] <- as.factor(mtcars[["cyl"]])
  mtcars[["gear"]] <- as.factor(mtcars[["gear"]])
})

transf <- teal_transform_module(
  label = "Transformator",
  ui = function(id) {
    ns <- NS(id)
    tags$div(
      numericInput(ns("n_rows"), "Number of rows to subset", value = 32, min = 1, max = 150, step = 1)
    )
  },
  server = function(id, data) {
    moduleServer(id, function(input, output, session) {
      reactive({
        within(data(),
               {
                 im_new <- head(iris, num_rows)
               },
               num_rows = input$n_rows
        )
      })
    })
  }
)

app <- init(
  data = data,
  modules = modules(
    tm_missing_data(parent_dataname = "mtcars", transformators = list(transf))
  )
)
if (interactive()) {
  shinyApp(app$ui, app$server)
}

image

@llrs-roche
Copy link
Contributor Author

llrs-roche commented Jan 24, 2025

This branch fixes tm_data_table, tm_variable_browser and tm_missing_data modules to display the output on several cases.
In addition, the tm_front_page module was changed. With this branch, tm_front_page by default doesn't show the right-panel about the datasets.

tm_front_page

tm_front_page: pkgload::run_example("man/tm_front_page.Rd") displays the output without the right-panel.

The other modules

I tested the three modules and the example_module on the app.

Examples

Previously they did not show the output. Currently they do:

  • example_module: pkgload::run_example("../teal/man/example_module.Rd")displays the output
  • tm_data_table: pkgload::run_example("man/tm_data_table.Rd") displays the output.
  • tm_variable_browser: pkgload::run_example("man/tm_variable_browser.Rd") displays the output.
  • tm_missing_data: pkgload::run_example("man/tm_missing_data.Rd") displays the output.

datanames = NULL

Testing modules using datanames = "NULL" to disable the right-panel:

Code reprex

Code output:

tmg_datanames_null

Code:

library("teal")
devtools::load_all(".")

# general example data
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
  mtcars <- mtcars
  
  iris[] <- lapply(iris, add_nas)
  mtcars[] <- lapply(mtcars, add_nas)
  mtcars[["cyl"]] <- as.factor(mtcars[["cyl"]])
  mtcars[["gear"]] <- as.factor(mtcars[["gear"]])
})

datanames <- NULL
app <- init(
  data = data,
  modules = modules(
    example_module(datanames = datanames),
    tm_front_page(datanames = datanames),
    tm_data_table(datanames = datanames),
    tm_variable_browser(datanames = datanames),
    tm_missing_data(parent_dataname = "mtcars", datanames = datanames)
  )
)
if (interactive()) {
  shinyApp(app$ui, app$server)
}

  • example_module: It doesn't show the right-panel
  • tm_front_page: It doesn't show the right-panel
  • tm_data_table: It doesn't show the right-panel
  • tm_variable_browser: It doesn't show the right-panel
  • tm_missing_data: It doesn't show the right-panel

From these modules only tm_missing_data has a parameter to pass transformers.
tm_missing_data with a transformer shows the right-panel with all the datasets:

tm_missing_data with transformer

Code output:

image

Code:

library("teal")
devtools::load_all(".")

# general example data
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
  mtcars <- mtcars
  
  iris[] <- lapply(iris, add_nas)
  mtcars[] <- lapply(mtcars, add_nas)
  mtcars[["cyl"]] <- as.factor(mtcars[["cyl"]])
  mtcars[["gear"]] <- as.factor(mtcars[["gear"]])
})

transf <- teal_transform_module(
  label = "Transformator",
  ui = function(id) {
    ns <- NS(id)
    tags$div(
      numericInput(ns("n_rows"), "Number of rows to subset", value = 32, min = 1, max = 150, step = 1)
    )
  },
  server = function(id, data) {
    moduleServer(id, function(input, output, session) {
      reactive({
        within(data(),
               {
                 im_new <- head(iris, num_rows)
               },
               num_rows = input$n_rows
        )
      })
    })
  }
)

datanames <- NULL
app <- init(
  data = data,
  modules = modules(
    tm_missing_data(parent_dataname = "mtcars", transformators = list(transf), datanames = datanames)
  )
)
if (interactive()) {
  shinyApp(app$ui, app$server)
}

datanames = "all"

Testing modules using datanames = "all" to show all the datasets on the right-panel:

Code

library("teal")
devtools::load_all(".")

# general example data
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
  mtcars <- mtcars
  
  iris[] <- lapply(iris, add_nas)
  mtcars[] <- lapply(mtcars, add_nas)
  mtcars[["cyl"]] <- as.factor(mtcars[["cyl"]])
  mtcars[["gear"]] <- as.factor(mtcars[["gear"]])
})

datanames <- "all"
app <- init(
  data = data,
  modules = modules(
    example_module(datanames = datanames),
    tm_front_page(datanames = datanames),
    tm_data_table(datanames = datanames),
    tm_variable_browser(datanames = datanames),
    tm_missing_data(parent_dataname = "mtcars", datanames = datanames)
  )
)
if (interactive()) {
  shinyApp(app$ui, app$server)
}

  • example_module: It shows the right-panel
  • tm_front_page: It shows the right-panel
  • tm_data_table: It shows the right-panel
  • tm_variable_browser: It shows the right-panel
  • tm_missing_data: It shows the right-panel.

@llrs-roche llrs-roche marked this pull request as ready for review January 24, 2025 14:43
Copy link
Contributor

github-actions bot commented Jan 24, 2025

Unit Tests Summary

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

Results for commit 9e6e609.

♻️ This comment has been updated with latest results.

Copy link
Contributor

github-actions bot commented Jan 24, 2025

Unit Test Performance Difference

Test Suite $Status$ Time on main $±Time$ $±Tests$ $±Skipped$ $±Failures$ $±Errors$
examples 💚 $5.68$ $-5.07$ $-1$ $+38$ $0$ $0$
shinytest2-tm_a_pca 💚 $122.53$ $-2.71$ $0$ $0$ $0$ $0$
shinytest2-tm_a_regression 💚 $54.10$ $-2.52$ $0$ $0$ $0$ $0$
shinytest2-tm_data_table 💚 $19.65$ $-1.53$ $0$ $0$ $0$ $0$
shinytest2-tm_file_viewer 💚 $30.09$ $-1.53$ $0$ $0$ $0$ $0$
shinytest2-tm_front_page 💚 $21.50$ $-1.15$ $0$ $0$ $0$ $0$
shinytest2-tm_g_association 💚 $32.94$ $-2.33$ $0$ $0$ $0$ $0$
shinytest2-tm_g_bivariate 💚 $76.75$ $-2.90$ $0$ $0$ $0$ $0$
shinytest2-tm_g_distribution 💚 $59.72$ $-1.73$ $0$ $0$ $0$ $0$
shinytest2-tm_g_scatterplot 💚 $74.90$ $-2.08$ $0$ $0$ $0$ $0$
shinytest2-tm_g_scatterplotmatrix 💚 $28.55$ $-1.36$ $0$ $0$ $0$ $0$
shinytest2-tm_misssing_data 💚 $49.32$ $-2.05$ $0$ $0$ $0$ $0$
shinytest2-tm_outliers 💚 $115.79$ $-4.38$ $0$ $0$ $0$ $0$
shinytest2-tm_t_crosstable 💚 $32.74$ $-1.23$ $0$ $0$ $0$ $0$
shinytest2-tm_variable_browser 💚 $59.88$ $-1.96$ $0$ $0$ $0$ $0$
Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
examples 💚 $1.85$ $-1.76$ example_add_facet_labels.Rd

Results for commit 5195a4d

♻️ This comment has been updated with latest results.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: missing_data, data_table and variable_browser are not displating output
2 participants