Fix outliers module

[nikolas-burkoff]

Closes #255 - also fixes a bug where app crashed if the categorical_var is not specified

Test on vignette, example in the issue and also test by not specifying categorical_var

For some reason the merge was being triggered on load (and when user deselects the dataset) whilst selector_list()$outlier_var was NULL.

I wonder if there are any other places in the code base this will affect - e.g. when having a list of data_extract_spec in a single data_extract_ui and deselecting the dataset?

@mhallal1, @gogonzo I don't really understand what's happening here and maybe there should be a fix in teal.devel instead of the band-aid here?

[github-actions]

Code Coverage Summary

Filename                      Stmts    Miss  Cover    Missing
--------------------------  -------  ------  -------  --------------------------------------------------------
R/tm_a_pca.R                    758     758  0.00%    69-956
R/tm_a_regression.R             675     675  0.00%    88-849
R/tm_data_table.R               165     165  0.00%    143-371
R/tm_file_viewer.R              168     168  0.00%    38-236
R/tm_g_association.R            254     254  0.00%    83-389
R/tm_g_bivariate.R              582     424  27.15%   105-605, 653, 659, 663, 774, 778, 780, 791, 809, 829-851
R/tm_g_distribution.R           891     891  0.00%    102-1136
R/tm_g_response.R               272     272  0.00%    83-412
R/tm_g_scatterplot.R            498     498  0.00%    137-707
R/tm_g_scatterplotmatrix.R      219     201  8.22%    76-311, 364, 378
R/tm_missing_data.R             925     925  0.00%    46-1132
R/tm_outliers.R                 915     915  0.00%    71-1115
R/tm_t_crosstable.R             201     201  0.00%    77-315
R/tm_variable_browser.R         715     715  0.00%    44-1154
R/utils.R                        60      60  0.00%    70-202
R/zzz.R                           1       1  0.00%    2
TOTAL                          7299    7123  2.41%

Results for commit: 5e008f64f3f876551d20b2c9500b40440b5c643d

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

[github-actions]

Unit Tests Summary

  1 files    3 suites   0s ⏱️
  9 tests   9 ✔️ 0 💤 0 ❌
42 runs  42 ✔️ 0 💤 0 ❌

Results for commit 8043658.

♻️ This comment has been updated with latest results.

[Polkas]

The error occurs here https://github.com/insightsengineering/teal.devel/blob/1750c9d4d5068edc94d0426617662226c2dc8ee9/R/merge_datasets.R#L50. I am investigating further.

[Polkas]

The problem is here https://github.com/insightsengineering/teal.devel/blob/1750c9d4d5068edc94d0426617662226c2dc8ee9/R/data_extract_module.R#L394-L407. For sure is not in the scope of this PR.

[Polkas]

@mhallal1 here we have a problem with merge module/ data extract. Looks to be a global problem.

[Polkas]

I am finally sure what is the problem.
When we use insertUI with instanuos option the UI is visible immediately. However inputs inserted (by insertUI) are not yet added to client. So we have to wait for the next reactive round. Thus the first reactive round return NULL for inputs.
This is a global NEST thing as it comes from teal::srv_teal function.
Code: https://github.com/insightsengineering/teal/blob/27d35cfdce4dbae9b1560d5be84c9801f48d9c2a/R/module_teal.R#L205

[nikolas-burkoff]

I am finally sure what is the problem.

Good spot!

[Polkas]

browser in both apps.

inputs should be available from the first line of code in server like here (example shiny app):

#
# This is a Shiny web application. You can run the application by clicking
# the 'Run App' button above.
#
# Find out more about building applications with Shiny here:
#
#    http://shiny.rstudio.com/
#

library(shiny)

# Define UI for application that draws a histogram
ui <- fluidPage(

    # Application title
    titlePanel("Old Faithful Geyser Data"),

    # Sidebar with a slider input for number of bins 
    sidebarLayout(
        sidebarPanel(
            sliderInput("bins",
                        "Number of bins:",
                        min = 1,
                        max = 50,
                        value = 30)
        ),

        # Show a plot of the generated distribution
        mainPanel(
           plotOutput("distPlot")
        )
    )
)

# Define server logic required to draw a histogram
server <- function(input, output) {
# inputs are available
browser()
    output$distPlot <- renderPlot({
        # generate bins based on input$bins from ui.R
        x    <- faithful[, 2]
        bins <- seq(min(x), max(x), length.out = input$bins + 1)

        # draw the histogram with the specified number of bins
        hist(x, breaks = bins, col = 'darkgray', border = 'white')
    })
}

# Run the application 
shinyApp(ui = ui, server = server)

However when we use insertUI (even with immediate option) with shiny inputs then we have to wait for second reactive cycle to see inputs added by insertUI.

#
# This is a Shiny web application. You can run the application by clicking
# the 'Run App' button above.
#
# Find out more about building applications with Shiny here:
#
#    http://shiny.rstudio.com/
#

library(shiny)

# Define UI for application that draws a histogram
ui <- fluidPage(

    # Application title
    titlePanel("Old Faithful Geyser Data"),

    # Sidebar with a slider input for number of bins
    sidebarLayout(
        sidebarPanel(
            sliderInput("bins",
                        "Number of bins:",
                        min = 1,
                        max = 50,
                        value = 30),
        div(id = "input22")
        ),
        # Show a plot of the generated distribution
        mainPanel(
           plotOutput("distPlot")
        )
    )
)

# Define server logic required to draw a histogram
server <- function(input, output, session) {

    insertUI("#input22", immediate = TRUE, ui = textInput(session$ns("xx"), "xx", "xxx"))

    output$distPlot <- renderPlot({
        ## input xx visible in the second reactive cycle.
        browser()
        # generate bins based on input$bins from ui.R
        x    <- faithful[, 2]
        bins <- seq(min(x), max(x), length.out = input$bins + 1)

        # draw the histogram with the specified number of bins
        hist(x, breaks = bins, col = 'darkgray', border = 'white')
    })
}

# Run the application
shinyApp(ui = ui, server = server)

[Polkas]

Looks like this https://github.com/insightsengineering/teal/blob/1ae9092332afe97b4159a3ed609681c8796a4b81/R/module_teal_with_splash.R#L60-L70
shoudl be transfered here
https://github.com/insightsengineering/teal/blob/1ae9092332afe97b4159a3ed609681c8796a4b81/R/init.R#L186

then we could resolve ddl args on ui level and insertUI will not be needed?

[gogonzo]

Still not sure why this works. Custom module is "inserted" by the teal and input is available immediately. No req's no validation.

library(teal)
library(scda)
ADSL <- synthetic_cdisc_data("latest")$adsl

app <- init(
  data = cdisc_data(cdisc_dataset("ADSL", ADSL)),
  modules = root_modules(
    module(
      label = "test1",
      filters = "all",
      ui = function(id, ...) {
        ns <- NS(id)
        standard_layout(
          encoding = sliderInput(ns("bins"), "Number of bins:", min = 1, max = 50, value = 30), 
          output = white_small_well(plotOutput(ns("plot")))
        )
      },
      server = function(input, output, session, datasets) {
        output$plot <- renderPlot({
          cat("\ninput$bins is:", input$bins)
          x <- datasets$get_data("ADSL")$BMRKR1
          bins <- seq(min(x), max(x), length.out = input$bins + 1)
          hist(x, breaks = bins, col = 'darkgray', border = 'white')
        })
      }
    )
  )
)
runApp(app)

Console output

input$bins is: 30^C

[Polkas]

@gogonzo if we put cat 2 lines higher you will see the problem.

...
      server = function(input, output, session, datasets) {
        cat("\ninput$bins is:", input$bins)
        output$plot <- renderPlot({
...

in normal/regular server function input is visible from 1 line of server function, here is not.

[gogonzo]

@gogonzo if we put cat 2 lines higher you will see the problem.

...
      server = function(input, output, session, datasets) {
        cat("\ninput$bins is:", input$bins)
        output$plot <- renderPlot({
...

in normal/regular server function input is visible from 1 line of server function, here is not.

Yes, you are right. The bounty is not, how to render modules without insertUI but simply by calling modules (their ui and server).

[Polkas]

After discussion with @pawelru , I give the approve. I will create a separate issue concerning the teal problem.