Feature branch PR #603
Merged
Feature branch PR #603
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ui add in ui active
Signed-off-by: André Veríssimo <[email protected]>
|
Code Coverage SummaryDiff against mainResults for commit: cfb750a Minimum allowed coverage is ♻️ This comment has been updated with latest results |
#602) # Pull Request Companion of insightsengineering/teal#1275 Note: There's a similar solution that is cleaner. I'll push it on Monday #### Changes description - Keeps track of `remove observers` on filter panel - Destroys `remove observer` when clearing filter panel Co-authored-by: go_gonzo <[email protected]>
Unit Tests Summary 1 files 29 suites 23s ⏱️ Results for commit cfb750a. ♻️ This comment has been updated with latest results. |
WIP. Still testing with more filter option Companion of insightsengineering/teal#1275 #### Changes description - Removes all observeEvents generated from `FilterData` when `finalize` method is called. #### How to test - Override `observeEvent` in `{teal.slice}` in order to keep track of all that are created - Stores observers in `.tmp_list` on `.GlobalEnv` - Place `browser()` call somewhere with access to `FilterData` object - Run snippet at bottom that shows count of observers that have not been destroyed - These are shown in order of creation `<order>_<parent r6 class>_<memory address>` - Run `finalize()` - Run snippet again <details> <summary>Example teal app</summary> ```r .tmp_list <- rlang::new_environment() options( teal.log_level = "INFO", teal.show_js_log = TRUE, # teal.bs_theme = bslib::bs_theme(version = 5), shiny.bookmarkStore = "server" ) pkgload::load_all("../teal.slice") pkgload::load_all("../teal") data <- teal::teal_data_module( ui = function(id) { ns <- shiny::NS(id) shiny::tagList( shiny::checkboxGroupInput( ns("datasets"), "Datasets", choices = c("ADSL", "ADTTE", "iris", "CO2", "miniACC"), selected = c("ADSL", "ADTTE", "iris", "CO2") ), shiny::actionButton(ns("submit"), label = "Submit") ) }, server = function(id, ...) { shiny::moduleServer(id, function(input, output, session) { code <- list( ADSL = expression( ADSL <- teal.data::rADSL ), ADTTE = expression({ ADTTE <- teal.data::rADTTE ADTTE$CNSRL <- as.logical(ADTTE$CNSR) }), iris = expression( iris <- iris ), CO2 = expression({ CO2 <- CO2 factors <- names(Filter(isTRUE, vapply(CO2, is.factor, logical(1L)))) CO2[factors] <- lapply(CO2[factors], as.character) }), miniACC = expression({ data( "miniACC", package = "MultiAssayExperiment", envir = environment(), overwrite = TRUE ) miniACC <- miniACC }) ) datasets <- reactive(input$datasets) shiny::eventReactive(input$submit, { code_to_eval <- do.call(c, code[datasets()]) data <- teal.code::eval_code(teal.data::teal_data(), code_to_eval) join_keys(data) <- default_cdisc_join_keys[datasets()] teal.data::datanames(data) <- datasets() data }) }) }, once = FALSE ) teal::init( data = data, modules = teal::modules( teal::example_module(label = "A"), teal::example_module(label = "B") ), filter = teal::teal_slices( # FilterRange teal.slice::teal_slice("ADSL", "AGE", selected = c(18L, 65L)), # FilterExpr teal_slice( dataname = "ADSL", id = "Female adults", expr = "SEX == 'F' & AGE >= 18", title = "Female adults" ), # FilterDatetime teal_slice( dataname = "ADTTE", varname = "ADTM", id = "Analysis DTM", selected = c("2019-03-25 07:06:18", "2020-01-22 15:03:58"), title = "Female adults" ), # FilterDate with LSTALVDT teal_slice( dataname = "ADSL", varname = "LSTALVDT", id = "Last Alive Date", selected = c("2022-02-14", "2022-11-24"), title = "Last Alive Date" ), # FilterEmpty # FilterLogical with CNSRL teal_slice( dataname = "ADTTE", varname = "CNSRL", id = "Censored", selected = TRUE, title = "Censored" ), module_specific = TRUE, teal.slice::teal_slice("ADSL", "SEX") ), title = "yada" ) |> shiny::runApp() ``` </details> <details> <summary>"observeEvent" override</summary> ```r observeEvent = function(eventExpr, handlerExpr, ... ) { logger::log_info("yada") rlang::enquo(eventExpr) rlang::enquo(handlerExpr) obs <- do.call( shiny::observeEvent, list( eventExpr = rlang::enquo(eventExpr), handlerExpr = rlang::enquo(handlerExpr), ... ), envir = parent.frame() ) # Create a temporary list to store observers and parent objects if (is.null(.GlobalEnv$.tmp_list)) .GlobalEnv$.tmp_list <- rlang::new_environment() self <- parent.env(parent.env(parent.frame()))$self obj_addr <- rlang::obj_address(self) |> as.character() |> stringr::str_replace("0x", "") obj_addr <- paste0(class(self)[1], "_", obj_addr) .tmp_list[["objects"]] <- c( list(), .tmp_list[["objects"]], setNames(list(self), obj_addr) ) .tmp_list[[sprintf("%03d_%s", length(.tmp_list[["objects"]]), obj_addr)]] <- c( list(), .tmp_list[[obj_addr]], list(obs) ) obs } ``` </details> <details> <summary>Snippet to analyse ".tmp_list"</summary> ```r ls(.tmp_list) |> purrr::keep(~!grepl("^objects$", .x)) |> vapply( \(x) { sum( vapply( .tmp_list[[x]], \(.x) isFALSE(.x$.destroyed), integer(1L) ) ) }, integer(1L) ) |> as.list() |> jsonlite::toJSON(pretty = TRUE, auto_unbox = TRUE) ``` </details>  --------- Co-authored-by: go_gonzo <[email protected]>
averissimo
reviewed
Aug 5, 2024
…ule (#604) links to insightsengineering/teal#1282 Fixes the +/- icon behavior on the filter panel. Please checkout to the `fix-add-remove-ui@669_insertUI@main` in both `teal` and `teal.slice` In `teal.slice` it just reverts one commit to the working version of the show/hide logic. When the data changes using `teal_data_module` the filter panel observers are being called once again thus running the scope of the observers as many times the data changes. The fix is to reinitialize the filter panel with new NS so the observers are not connected to the active UI anymore. ### Example app to test ```r pkgload::load_all("teal.slice") pkgload::load_all("teal") library(MultiAssayExperiment) data <- teal::teal_data_module( ui = function(id) { ns <- shiny::NS(id) shiny::tagList( shiny::textInput(ns("username"), label = "Username"), shiny::passwordInput(ns("password"), label = "Password"), shiny::actionButton(ns("submit"), label = "Submit") ) }, server = function(id, ...) { shiny::moduleServer(id, function(input, output, session) { shiny::eventReactive(input$submit, { data <- teal.data::teal_data() |> within( { logger::log_debug("Loading data...") iris <- iris data("miniACC", package = "MultiAssayExperiment", envir = environment()) }, username = input$username, password = input$password ) data }) }) }, once = FALSE ) app <- init( data = data, modules = example_module() ) shiny::runApp(app$ui, app$server) ``` --------- Signed-off-by: Vedha Viyash <[email protected]> Co-authored-by: Dawid Kałędkowski <[email protected]>
Unit Test Performance DifferenceAdditional test case details
Results for commit d831361 ♻️ This comment has been updated with latest results. |
Fix Bug found by @vedhav when reloading data.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ui/srv_addtoui/srv_activeself$finalizemethod to destroy all the observers and inputs (needed after data reloading)