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

904 teal_data in teal_module #924

Merged
merged 50 commits into from
Nov 22, 2023
Merged

Conversation

chlebowa
Copy link
Contributor

@chlebowa chlebowa commented Sep 25, 2023

Closes #904

Added as_tdata function to convert the data object received by modules from teal_data to tdata class.
Modified srv_nested_tabs.teal_module now returns a teal_data.

To make a module handle a teal_data object without (much) modification, the incoming data argument should be downgraded to the old class.

      data <- as_tdata(data)

Also modified example_module to handle teal_data objects.

@chlebowa chlebowa added the core label Sep 25, 2023
@chlebowa chlebowa marked this pull request as ready for review September 25, 2023 14:56
@chlebowa chlebowa linked an issue Sep 25, 2023 that may be closed by this pull request
R/module_nested_tabs.R Outdated Show resolved Hide resolved
R/module_nested_tabs.R Outdated Show resolved Hide resolved
R/module_nested_tabs.R Outdated Show resolved Hide resolved
@gogonzo gogonzo changed the base branch from refactor to teal_data@main October 4, 2023 12:41
Copy link
Contributor

@gogonzo gogonzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

example_module doesn't need downgraded data. .tdata_downgrade isn't needed at all - why would someone downgrade module which works with teal_data?

If you apply my comments and fix example_module to be as follows - everything should work (tested already on my side)

example_module
example_module <- function(label = "example teal module", datanames = "all") {
  checkmate::assert_string(label)
  module(
    label,
    ui = function(id) {
      ns <- NS(id)
      teal.widgets::standard_layout(
        output = dataTableOutput(ns("table")),
        encoding = div(
          selectInput(ns("dataname"), "Choose a dataset", choices = NULL),
          teal.widgets::verbatim_popup_ui(ns("rcode"), "Show R code")
        )
      )
    },
    server = function(id, data) {
      checkmate::assert_class(data, "reactive")
      moduleServer(id, function(input, output, session) {
        observeEvent(data(), {
          updateSelectInput(session, "dataname", choices = teal.data::datanames(data()))
        })

        output$table <- renderDataTable(data()[[input$dataname]])

        teal.widgets::verbatim_popup_srv(
          id = "rcode",
          verbatim_content = reactive(teal.code::get_code(data())),
          title = "Association Plot"
        )
      })
    },
    datanames = datanames
  )
}

R/module_nested_tabs.R Outdated Show resolved Hide resolved
R/dummy_functions.R Outdated Show resolved Hide resolved
R/module_nested_tabs.R Outdated Show resolved Hide resolved
Copy link
Contributor

github-actions bot commented Nov 21, 2023

badge

Code Coverage Summary

Filename                         Stmts    Miss  Cover    Missing
-----------------------------  -------  ------  -------  -------------------------------------------------------------------------------------------------------------------------------
R/dummy_functions.R                 92      50  45.65%   9-71
R/get_rcode_utils.R                 46       1  97.83%   49
R/include_css_js.R                  24       0  100.00%
R/init.R                            79      25  68.35%   140, 165-186, 218-220, 222-224, 226-227
R/landing_popup_module.R            25      25  0.00%    61-87
R/module_filter_manager.R          107      29  72.90%   62-70, 79-84, 228, 233-246
R/module_nested_tabs.R             163      14  91.41%   72, 119, 138-145, 163, 216, 238, 271
R/module_snapshot_manager.R        209     157  24.88%   87-99, 127-136, 140-152, 154-161, 168-182, 186-188, 190-195, 198-208, 211-227, 236-251, 265-288, 291-302, 305-311, 325, 346-369
R/module_tabs_with_filters.R        73       2  97.26%   95, 140
R/module_teal_with_splash.R        119      11  90.76%   72, 82-90, 121
R/module_teal.R                    141       8  94.33%   68, 71, 158-159, 165, 195, 203-204
R/modules_debugging.R               18      18  0.00%    25-44
R/modules.R                        143      26  81.82%   119, 132, 226-229, 243-248, 259-263, 378-421
R/reporter_previewer_module.R       18       2  88.89%   26, 30
R/show_rcode_modal.R                20      20  0.00%    16-37
R/tdata.R                           53       1  98.11%   161
R/teal_data_module.R                 6       0  100.00%
R/teal_reporter.R                   60       5  91.67%   65, 116-117, 120, 137
R/teal_slices-store.R               25       0  100.00%
R/teal_slices.R                     59      12  79.66%   135-148
R/utils.R                          110      27  75.45%   113-140
R/validate_inputs.R                 32       0  100.00%
R/validations.R                     58      37  36.21%   109-371
R/zzz.R                             11       7  36.36%   3-14
TOTAL                             1691     477  71.79%

Diff against main

Filename                  Stmts    Miss  Cover
----------------------  -------  ------  -------
R/module_nested_tabs.R      -13       0  -0.63%
R/tdata.R                   +14       0  +0.68%
TOTAL                        +1       0  +0.02%

Results for commit: 959eddd

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@chlebowa
Copy link
Contributor Author

example_module doesn't need downgraded data. .tdata_downgrade isn't needed at all - why would someone downgrade module which works with teal_data?

As previously discussed, .tdata_downgrade may be necessary for external teal modules that will NOT wirk with teal_data.
Putting it in example_module was a proof of concept and an example to be discussed. See opening post.

@chlebowa chlebowa changed the base branch from main to refactor November 21, 2023 15:39
Aleksander Chlebowski added 2 commits November 21, 2023 16:43
@chlebowa chlebowa requested a review from gogonzo November 21, 2023 15:44
@gogonzo gogonzo self-assigned this Nov 22, 2023
Copy link
Contributor

@gogonzo gogonzo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean!

@chlebowa chlebowa merged commit e7ab218 into refactor Nov 22, 2023
@chlebowa chlebowa deleted the 904_tdata_in_modules@refactor branch November 22, 2023 08:19
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.

[Feature request] Rework <teal_module>$server to get teal_data object
3 participants