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

[Bug]: Filtered vs unfiltered counts and two-tone coloring is gone? #396

Closed
3 tasks done
lcd2yyz opened this issue Jul 21, 2023 · 5 comments
Closed
3 tasks done

[Bug]: Filtered vs unfiltered counts and two-tone coloring is gone? #396

lcd2yyz opened this issue Jul 21, 2023 · 5 comments
Labels
bug Something isn't working core

Comments

@lcd2yyz
Copy link

lcd2yyz commented Jul 21, 2023

What happened?

As title states, the hierarchical filter mechanism to show/visualize filtered vs unfiltered data is now missing from filter panel.
Tested with main branch on both teal and teal.slice
image

sessionInfo()

No response

Relevant log output

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct.

Contribution Guidelines

  • I agree to follow this project's Contribution Guidelines.

Security Policy

  • I agree to follow this project's Security Policy.
@lcd2yyz lcd2yyz added bug Something isn't working core labels Jul 21, 2023
@gogonzo
Copy link
Contributor

gogonzo commented Jul 21, 2023

@lcd2yyz the default value of the teal_slice(count_type = "none", ...) so by default we don't start with hierarchical counts. One can enable this by setting teal_slice(count_type = "all"), reason we changed it is that MultiAssayExperiment has some automatic "refreshing" of the object when applying subjects filter, and R throws red error. We wrote this in the documentation of count_type argument

#' and unfiltered dataset. Note, that issues were reported when using this option with `MultiAssayExperiment`.

funny_module <- function(label = "Filter states", datanames = "all") {
  checkmate::assert_string(label)
  module(
    label = label,
    filters = datanames,
    ui = function(id, ...) {
      ns <- NS(id)
      div(
        h2("The following filter calls are generated:"),
        verbatimTextOutput(ns("filter_states")),
        verbatimTextOutput(ns("filter_calls")),
        actionButton(ns("reset"), "reset_to_default")
      )
    },
    server = function(input, output, session, data, filter_panel_api) {
      checkmate::assert_class(data, "tdata")
      observeEvent(input$reset, set_filter_state(filter_panel_api, default_filters))
      output$filter_states <- renderText({
        logger::log_trace("rendering text1")
        filter_panel_api |>
          get_filter_state() |>
          format(trim = FALSE)
      })
      output$filter_calls <- renderText({
        logger::log_trace("rendering text2")
        attr(data, "code")()
      })
    }
  )
}
options(teal.log_level = "TRACE", teal.show_js_log = TRUE)
# options("teal.bs_theme" = bslib::bs_theme(version = 5))
# options(shiny.trace = TRUE)
devtools::load_all("teal.slice")
devtools::load_all("teal")
library(scda)
library(SummarizedExperiment)

default_filters <- teal_slices(
  teal_slice(dataname = "MAE", datalabel = "subjects", varname = "AGE"),
  teal_slice(dataname = "MAE", datalabel = "hd1", arg = "select", varname = "low_depth_flag", selected = TRUE),
  teal_slice(dataname = "MAE", datalabel = "hd1", arg = "select", varname = "tech_failure_flag"),
  include_varnames = list(MAE = c("AGE", "ARM")),
  count_type = "all"
)

MAE <- hermes::multi_assay_experiment

data <- teal.data::teal_data(teal.data::dataset("MAE", MAE))

app <- init(
  data = data,
  modules = list(funny_module(), funny_module("module2")),
  filter = default_filters
)

runApp(app)
Skärmavbild 2023-07-21 kl  08 06 17

P.S. Do you want count_type = "all" by default?

@lcd2yyz
Copy link
Author

lcd2yyz commented Jul 21, 2023

Ah thanks for explaining. Once I updated to count_type = "all" it showed up nicely.
Yes, I consider to be a key feature of the new filter panel, so I was expecting it to be the default.

Does it just not work at all with MAE objects? Or does it work somewhat, just not reliably? Is this some special handling we can do, for example, this count_type argument have no effect if input is MAE, and it will always just display the unfiltered counts?

@gogonzo
Copy link
Contributor

gogonzo commented Jul 24, 2023

@lcd2yyz seems to not work with MAE at all. If I had more time I would investigate this further but it's not a few hours issue from what I've seen. Initially I set count_type = "none" in MAEFIlteredData but it failed the app when other datasets were included with count_type = "all" as all datasets should have the same teal_slices attributes.

One solution to this problem is to implement this in teal::init to set default count_type to "all" when all data objects are data.frame or "none" otherwise. But I'm not sure if it's a right way - to make count_type conditional on some other objects

@lcd2yyz
Copy link
Author

lcd2yyz commented Jul 28, 2023

Maybe we should label this count_type argument to be "experimental" then?
The current documentation might be a bit misleading that it only mentioned not working for MAE, however, I'm not sure if we tested on other data structures. So in the rare event that advanced users create some customized TealDataset loader for some special data class, user would not expect this feature to work.
I'm thinking to enable set default count_type = 'all' in teal::init for all. However, add a check in teal_slices where if the input data object is not data.frame, display a warning to say, this feature is currently only available/tested with normal data frames, and is ignored (effectively reset count_type = 'none' if not data frame).

Also can we add a vignette to introduce this new filter panel, with examples of all the various teal_slices settings. Something along the lines of "Customizing Teal filter panel options".

@gogonzo
Copy link
Contributor

gogonzo commented May 28, 2024

closing in favour of #596

@gogonzo gogonzo closed this as completed May 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working core
Projects
None yet
Development

No branches or pull requests

2 participants