insightsengineering / teal

Exploratory Web Apps for Analyzing Clinical Trial Data
https://insightsengineering.github.io/teal/
Other
176 stars 35 forks source link

Extend the table fetcher to handle DT tables too #1210

Closed vedhav closed 4 months ago

vedhav commented 5 months ago

Extend the get_active_module_tws_output to also work for DT::DTOutput which is now renamed to get_active_module_table_output along with get_active_module_plot_output

Example to test:

devtools::load_all("../teal")
library(teal.modules.clinical)

data <- teal_data()
data <- within(data, {
  ADSL <- teal.data::rADSL
  ADLB <- teal.data::rADLB
})

datanames <- c("ADSL", "ADLB")
teal.data::datanames(data) <- datanames
teal.data::join_keys(data) <- teal.data::default_cdisc_join_keys[datanames]

app_driver <- TealAppDriver$new(
  data = data,
  modules = tm_t_pp_laboratory(
    label = "Vitals",
    dataname = "ADLB",
    patient_col = "USUBJID",
    paramcd = teal.transform::choices_selected(
      choices = teal.transform::variable_choices(data[["ADLB"]], "PARAMCD"),
      selected = "PARAMCD"
    ),
    param = teal.transform::choices_selected(
      choices = teal.transform::variable_choices(data[["ADLB"]], "PARAM"),
      selected = "PARAM"
    ),
    timepoints = teal.transform::choices_selected(
      choices = teal.transform::variable_choices(data[["ADLB"]], "ADY"),
      selected = "ADY"
    ),
    anrind = teal.transform::choices_selected(
      choices = teal.transform::variable_choices(data[["ADLB"]], "ANRIND"),
      selected = "ANRIND"
    ),
    aval_var = teal.transform::choices_selected(
      choices = teal.transform::variable_choices(data[["ADLB"]], "AVAL"),
      selected = "AVAL"
    ),
    avalu_var = teal.transform::choices_selected(
      choices = teal.transform::variable_choices(data[["ADLB"]], "AVALU"),
      selected = "AVALU"
    )
  )
)

app_driver$get_active_module_table_output("lab_values_table", 2)
github-actions[bot] commented 5 months ago

badge

Code Coverage Summary

Filename                          Stmts    Miss  Cover    Missing
------------------------------  -------  ------  -------  --------------------------------------------------------------------------------------------------------------------------------------------------
R/TealAppDriver.R                   311     311  0.00%    43-647
R/dummy_functions.R                  36      25  30.56%   21-37, 40-47
R/get_rcode_utils.R                  31       1  96.77%   50
R/include_css_js.R                   22      17  22.73%   12-38, 76-82
R/init.R                             86      31  63.95%   108-115, 161-162, 164, 176-197, 228-229, 231
R/landing_popup_module.R             25      25  0.00%    61-87
R/module_bookmark_manager.R         158     125  20.89%   42-43, 57-59, 70-83, 93-143, 148-149, 189, 224-301
R/module_filter_manager.R            84      19  77.38%   38-42, 157, 162-175
R/module_nested_tabs.R              161      60  62.73%   39-112, 128, 180, 202, 224, 232, 236
R/module_snapshot_manager.R         241     178  26.14%   95-107, 136-139, 143-144, 159-169, 173-188, 190-198, 205-220, 224-228, 230-236, 239-252, 255-273, 282-298, 313-336, 339-350, 353-359, 373, 394-418
R/module_tabs_with_filters.R         76      33  56.58%   33-68, 100, 116
R/module_teal.R                     113      79  30.09%   52-119, 150-151, 157-160, 171, 184-215
R/module_teal_with_splash.R         114      34  70.18%   60-95, 110, 131, 197-198
R/module_wunder_bar.R                60      39  35.00%   23-41, 55-64, 68-77
R/modules.R                         159      26  83.65%   127-130, 147-151, 206-209, 291-292, 344, 356-364, 418-421
R/reporter_previewer_module.R        19       2  89.47%   30, 34
R/show_rcode_modal.R                 19      19  0.00%    17-36
R/tdata.R                            53       1  98.11%   154
R/teal_data_module-eval_code.R       27       0  100.00%
R/teal_data_module-within.R           7       0  100.00%
R/teal_data_module.R                  6       0  100.00%
R/teal_reporter.R                    68       6  91.18%   69, 77, 125-126, 129, 146
R/teal_slices-store.R                29       0  100.00%
R/teal_slices.R                      59      12  79.66%   137-150
R/utils.R                           173       1  99.42%   255
R/validate_inputs.R                  32       0  100.00%
R/validations.R                      58      37  36.21%   110-377
R/zzz.R                              12       8  33.33%   3-15
TOTAL                              2239    1089  51.36%

Diff against main

Filename             Stmts    Miss  Cover
-----------------  -------  ------  --------
R/TealAppDriver.R       -4      -4  +100.00%
TOTAL                   -4      -4  +0.09%

Results for commit: 2ebe93e77225e636e5938f5b406e7bf154743e65

Minimum allowed coverage is 80%

:recycle: This comment has been updated with latest results

github-actions[bot] commented 5 months ago

Unit Tests Summary

  1 files   30 suites   2m 48s :stopwatch: 240 tests 240 :white_check_mark: 0 :zzz: 0 :x: 505 runs  505 :white_check_mark: 0 :zzz: 0 :x:

Results for commit 2ebe93e7.

:recycle: This comment has been updated with latest results.

vedhav commented 5 months ago

Yeah, true. This should be get_active_module_table_output, And, it's companion get_active_module_plot_output which will slowly extend to other plot types if observed.

gogonzo commented 5 months ago
app_driver$get_active_module_table_output(
  table_id = "my_table-table-with-settings" 
)

Function name changed so we'll need to update test everywhere downstream. Why not to apply my suggestion . This should be relatively straightfoward.

app_driver$get_active_module_table_output(
  table_id = "my_table-table-with-settings" 
)

While "my_table-table-with-settings" theoreticaly could be handled by ns_pws (same as pws_des_input) but it is simple enoygh that I could write it manually.

vedhav commented 5 months ago

I am also okay with this change. But, I would still prefer to have the table-with-settings in once place for two reasons:

  1. ~All the input ids used in our teal module tests can be directly found in the module code. So, I would like to keep it that way if possible. As, it is easy to search for "table" in the code with teal.widgets::table_with_settings_ui(ns("table")).~ No, some modules are already using namespace ids where the namespace comes from teal.widgets
  2. Changing this class in the future will be easy if it is in one place.
vedhav commented 5 months ago

Okay, I already see that some of the ids are implemented as you suggested. So, I'm onboard. I'll remove this logic from there.