Skip to content

Restore gt_summary and allow for general usage#1004

Open
averissimo wants to merge 42 commits into
redesign_extraction@mainfrom
999-tm_gtsummary
Open

Restore gt_summary and allow for general usage#1004
averissimo wants to merge 42 commits into
redesign_extraction@mainfrom
999-tm_gtsummary

Conversation

@averissimo

@averissimo averissimo commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Pull Request

Changes description

  • Creates framework to use for gt-type data calls
  • Implements modules
    • tm_gtsummary with gtsummary backend
    • tm_tbl_roche_summary
    • tm_tbl_listing
  • Discuss solution
  • Discuss removal of .fun argument in favor of static implementation only
  • Documentation (pending technical discussion)

Technical details

Creates internal functions called {tm/srv/ui}_gt_template which implement a very flexible approach to arbitrary named arguments (also known as dots ...).

Name dots are dynamically generated because there are no common parameters between target functions.

  • data_extract_spec (future teal.picks::picks()): Print them dynamically on encoding panel. It will use data from teal.data object
  • choices_selected (future teal.picsk::variables(): Print an optional select input on ecoding panel
  • Other named dots: Considered static arguments for target function and not rendered in UI

This assumes that any module needs at least 2 implemented functions tm_<module_name> srv_<module_name>_partial with an optional ui_<module_name>_partial

The partial allows for custom logic and setting custom and optional UI.

Note: no need of partial for crane::tbl_listing as it has no custom logic. tbl_summary and tbl_roche_summary need as we're using radio buttons and custom validation.

Below is an example of a partial for gt_summary::tbl_summary (must return a qenv-like object to keep with teal practices)

srv_gtsummary_partial <- function(id,
                                  data,
                                  .fun_quo,
                                  ...,
                                  summary_args_r) {
  moduleServer(id, function(input, output, session) {
    summary_args_processed <- reactive({
      tbl_summary_args <- req(summary_args_r()) # Arguments forwarded from the main server function (template)
      tbl_summary_args$missing <- input$missing # Additional argument from custom UI
      tbl_summary_args$percent <- input$percent # Additional argument from custom UI
      tbl_summary_args
    })

    validated_q <- reactive({ # Custom validation for gtsummary
      q <- req(data())
      summary_args <- req(summary_args_processed())
      validate(
        need(
          length(summary_args$include) != 0L && all(!summary_args$include %in% summary_args$by),
          "Variables to stratify with and variables to include should be different"
        ),
      )
      q
    })

    srv_gt_template_partial(
      id = id,
      data = validated_q,
      .fun_quo = .fun_quo,
      ...,
      summary_args_r = summary_args_processed
    )
  })
}

Example app

Note:

  1. Last module uses internal function that works just the same as tm_gtsummary (without validation)
    • Shows how flexible this implementation is
  2. requires teal.widgets@999-tm_gtsummary branch for tbl_listing module to work (it splits the table in 2)
pkgload::load_all("../teal.widgets")
pkgload::load_all("../teal.modules.general")

add_overall_decorator <- function(col_label = "**Overall**  \nN = {style_number(N)}",
                                  statistic = NULL,
                                  digits = NULL,
                                  last = TRUE,
                                  enable = TRUE) {
  teal_transform_module(
    label = "Overall",
    ui = function(id) {
      ns <- NS(id)
      tags$div(
        bslib::input_switch(
          ns("enable"),
          label = "Enable it?",
          value = enable
        ),
        bslib::input_switch(
          ns("last"),
          label = "Show as last column",
          value = last
        )
      )
    },
    server = function(id, data) {
      moduleServer(id, function(input, output, session) {
        teal.logger::log_shiny_input_changes(input, namespace = "teal.modules.gtsummary")
        observeEvent(input$enable, {
          shinyjs::toggleState("last", input$enable)
        })
        reactive({
          if (!input$enable) {
            data()
          } else {
            q <- within(data(),
              {
                table <- gtsummary::add_overall(table,
                  col_label = col_label,
                  statistic = statistic, last = last,
                  digits = digits
                )
              },
              last = input$last,
              statistic = rlang::enexpr(statistic),
              digits = rlang::enexpr(digits),
              col_label = col_label
            )
            validate_qenv(q)
            q
          }
        })
      })
    }
  )
}

data <- within(teal.data::teal_data(), {
  ADSL <- teal.data::rADSL
  ADTTE <- teal.data::rADTTE
})
join_keys(data) <- default_cdisc_join_keys[names(data)]
app <- init(
  data = data,
  modules = modules(
    tm_tbl_roche_summary(label = "Roche Summary (no arguments)"),
    tm_tbl_summary(label = "GT Summary (no arguments)"),
    tm_tbl_roche_summary(
      label = "Roche Summary",
      by = teal.picks::picks(
        datasets(c("ADSL", "ADTTE"), "ADTTE"),
        variables(c("SEX", "COUNTRY", "SITEID", "ACTARM", "CNSR", "PARAMCD"), "SEX")
      ),
      include = teal.picks::picks(
        datasets(c("ADSL", "ADTTE"), "ADSL"),
        variables(c("SITEID", "COUNTRY", "ACTARM", "SEX"), "SITEID", multiple = TRUE)
      ),
      decorators = list(table = add_overall_decorator())
    ),
    tm_tbl_summary(
      label = "GT Summary",
      by = teal.picks::picks(
        datasets(c("ADSL", "ADTTE"), "ADSL"),
        variables(c("SEX", "COUNTRY", "SITEID", "ACTARM"), "SEX")
      ),
      include = teal.picks::picks(
        datasets(c("ADSL", "ADTTE"), "ADSL"),
        variables(c("SITEID", "COUNTRY", "ACTARM"), "SITEID", multiple = TRUE)
      )
    ),
    tm_tbl_summary(
      label = "GT Summary (with tidyselect)",
      dataname = "ADSL",
      include = gtsummary::everything()
    ),
    tm_tbl_listing(
      label = "Table Listing", 
      dataname = "ADSL",
      decorators = list(listing = add_overall_decorator())
    ),
    tm_gt_template(
      label = "GT Template (with gtsumary::tbl_summary)",
      .fun = gtsummary::tbl_summary,
      by = teal.picks::picks(
        datasets("ADSL", "ADSL"),
        variables(c("SEX", "COUNTRY", "SITEID", "ACTARM"))
      ),
      include = teal.picks::picks(
        datasets("ADSL", "ADSL"),
        variables(c("SITEID", "COUNTRY", "ACTARM"), "SITEID", multiple = TRUE)
      ),
      missing = teal.picks::values(c("no", "ifany", "always"), "ifany", multiple = FALSE),
      percent = teal.picks::values(c("column", "row", "cell"), "column", multiple = FALSE)
    )
  )
) |> shiny::runApp()

@averissimo averissimo added the core label Jul 7, 2026
@averissimo
averissimo marked this pull request as ready for review July 8, 2026 13:46
@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Unit Tests Summary

    1 files     37 suites   27m 12s ⏱️
  686 tests   683 ✅ 1 💤 1 ❌ 1 🔥
1 318 runs  1 315 ✅ 1 💤 1 ❌ 1 🔥

For more details on these failures and errors, see this check.

Results for commit ab1c648.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Unit Test Performance Difference

Test Suite $Status$ Time on main $±Time$ $±Tests$ $±Skipped$ $±Failures$ $±Errors$
examples 💔 $0.61$ $+6.14$ $+3$ $-39$ $+1$ $0$
shinytest2-tm_a_pca 💔 $1.03$ $+230.04$ $+38$ $-11$ $0$ $0$
shinytest2-tm_a_regression 💔 $0.61$ $+91.06$ $+24$ $-7$ $0$ $0$
shinytest2-tm_data_table 💔 $0.37$ $+37.14$ $+6$ $-4$ $0$ $0$
shinytest2-tm_file_viewer 💔 $1.28$ $+44.56$ $+9$ $-4$ $0$ $0$
shinytest2-tm_front_page 💔 $0.43$ $+33.47$ $+6$ $-3$ $0$ $0$
shinytest2-tm_g_association 💔 $0.64$ $+48.28$ $+9$ $-4$ $0$ $0$
shinytest2-tm_g_bivariate 💔 $0.35$ $+105.60$ $+40$ $-4$ $0$ $0$
shinytest2-tm_g_distribution 💔 $0.24$ $+121.03$ $+17$ $-3$ $0$ $0$
shinytest2-tm_g_response 💔 $0.33$ $+50.82$ $+13$ $-4$ $0$ $0$
shinytest2-tm_g_scatterplot 💔 $0.45$ $+142.40$ $+30$ $-5$ $0$ $0$
shinytest2-tm_g_scatterplotmatrix 💔 $0.57$ $+67.18$ $+5$ $-4$ $0$ $0$
shinytest2-tm_missing_data 💔 $0.81$ $+69.32$ $+18$ $-4$ $0$ $0$
shinytest2-tm_outliers 💔 $0.97$ $+176.53$ $+51$ $-10$ $0$ $0$
shinytest2-tm_t_crosstable 💔 $0.75$ $+50.40$ $+5$ $-4$ $0$ $0$
shinytest2-tm_variable_browser 💔 $0.62$ $+99.97$ $+17$ $-6$ $0$ $0$
tm_a_regression 💚 $8.04$ $-1.05$ $0$ $0$ $0$ $0$
tm_gtsummary 👶 $+8.86$ $+27$ $0$ $0$ $+1$
utils 💔 $8.62$ $+3.39$ $0$ $0$ $0$ $0$
Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
examples 💔 $0.02$ $+2.18$ example_add_facet_labels.Rd
examples 👶 $+0.08$ example_tm_gtsummary.Rd
examples 👶 $+0.93$ example_tm_roche_summary.Rd
examples 👶 $+0.06$ example_tm_tbl_listing.Rd
shinytest2-tm_a_pca 💔 $0.08$ $+16.48$ e2e_tm_a_pca_Changing_output_encodings_for_plot_type_does_not_generate_errors.
shinytest2-tm_a_pca 💔 $0.08$ $+10.93$ e2e_tm_a_pca_Changing_output_encodings_of_NA_action_does_not_generate_errors.
shinytest2-tm_a_pca 💔 $0.08$ $+74.20$ e2e_tm_a_pca_Changing_output_encodings_of_font_size_does_not_generate_errors.
shinytest2-tm_a_pca 💔 $0.08$ $+35.56$ e2e_tm_a_pca_Changing_output_encodings_of_plot_type_hides_and_shows_options.
shinytest2-tm_a_pca 💔 $0.08$ $+15.56$ e2e_tm_a_pca_Changing_output_encodings_of_standardization_does_not_generate_errors.
shinytest2-tm_a_pca 💔 $0.08$ $+11.86$ e2e_tm_a_pca_Changing_output_encodings_of_tables_display_does_not_generate_errors.
shinytest2-tm_a_pca 💔 $0.08$ $+12.41$ e2e_tm_a_pca_Changing_output_encodings_of_theme_does_not_generate_errors.
shinytest2-tm_a_pca 💔 $0.08$ $+15.00$ e2e_tm_a_pca_Color_by_columns_data_extract_must_be_from_non_selected_variable_set.
shinytest2-tm_a_pca 💔 $0.10$ $+11.48$ e2e_tm_a_pca_Eigenvector_table_should_have_data_extract_selection_Murder_Assault_on_header.
shinytest2-tm_a_pca 💔 $0.08$ $+14.77$ e2e_tm_a_pca_Eigenvector_table_should_have_data_extract_selection_Murder_UrbanPop_on_header.
shinytest2-tm_a_pca 💔 $0.21$ $+11.79$ e2e_tm_a_pca_Module_is_initialised_with_the_specified_defaults_in_function_call.
shinytest2-tm_a_regression 💔 $0.11$ $+9.80$ e2e_tm_a_regression_Data_extract_spec_elements_are_initialized_with_the_default_values_specified_by_response_and_regressor_arg.
shinytest2-tm_a_regression 💔 $0.10$ $+9.90$ e2e_tm_a_regression_Data_parameter_and_module_label_is_passed_properly.
shinytest2-tm_a_regression 💔 $0.08$ $+14.27$ e2e_tm_a_regression_Outlier_definition_and_label_are_visible_by_default.
shinytest2-tm_a_regression 💔 $0.08$ $+9.91$ e2e_tm_a_regression_Outlier_definition_and_label_have_default_values_and_label_text.
shinytest2-tm_a_regression 💔 $0.08$ $+20.15$ e2e_tm_a_regression_Plot_type_has_7_specific_choices_changing_choices_does_not_throw_errors.
shinytest2-tm_a_regression 💔 $0.08$ $+11.72$ e2e_tm_a_regression_Plot_type_is_set_properly.
shinytest2-tm_a_regression 💔 $0.08$ $+15.30$ e2e_tm_a_regression_Unchecking_display_outlier_hides_outlier_label_and_definition.
shinytest2-tm_data_table 💔 $0.10$ $+8.22$ e2e_tm_data_table_Initializes_without_errors
shinytest2-tm_data_table 💔 $0.09$ $+8.25$ e2e_tm_data_table_Verify_checkbox_displayed_over_data_table
shinytest2-tm_data_table 💔 $0.09$ $+9.15$ e2e_tm_data_table_Verify_default_variable_selection_and_set_new_selection
shinytest2-tm_data_table 💔 $0.09$ $+11.52$ e2e_tm_data_table_Verify_module_displays_data_table
shinytest2-tm_file_viewer 💔 $0.50$ $+9.44$ e2e_tm_file_viewer_Initializes_without_errors_and_shows_files_tree_specified_in_input_path_argument
shinytest2-tm_file_viewer 💔 $0.25$ $+11.62$ e2e_tm_file_viewer_Shows_selected_image_file
shinytest2-tm_file_viewer 💔 $0.26$ $+11.63$ e2e_tm_file_viewer_Shows_selected_text_file
shinytest2-tm_file_viewer 💔 $0.26$ $+11.87$ e2e_tm_file_viewer_Shows_selected_url
shinytest2-tm_front_page 💔 $0.14$ $+8.10$ e2e_tm_front_page_Initializes_without_errors_and_check_html_elements
shinytest2-tm_front_page 💔 $0.14$ $+12.99$ e2e_tm_front_page_Verify_the_module_displays_metadata
shinytest2-tm_front_page 💔 $0.14$ $+12.38$ e2e_tm_front_page_Verify_the_module_displays_tables
shinytest2-tm_g_association 💔 $0.12$ $+13.46$ e2e_tm_g_association_Check_and_set_default_values_for_radio_buttons.
shinytest2-tm_g_association 💔 $0.16$ $+12.78$ e2e_tm_g_association_Data_extract_spec_elements_are_initialized_with_the_default_values_specified_by_ref_and_vars_arguments.
shinytest2-tm_g_association 💔 $0.24$ $+9.92$ e2e_tm_g_association_Data_parameter_and_module_label_is_passed_properly.
shinytest2-tm_g_association 💔 $0.12$ $+12.12$ e2e_tm_g_association_Module_plot_is_visible.
shinytest2-tm_g_bivariate 💔 $0.10$ $+25.01$ e2e_tm_g_bivariate_Coloring_options_are_hidden_when_coloring_is_toggled_off.
shinytest2-tm_g_bivariate 💔 $0.08$ $+29.30$ e2e_tm_g_bivariate_Facetting_options_are_hidden_when_facet_is_toggled_off.
shinytest2-tm_g_bivariate 💔 $0.09$ $+11.02$ e2e_tm_g_bivariate_Module_is_initialised_with_the_specified_defaults.
shinytest2-tm_g_bivariate 💔 $0.08$ $+40.28$ e2e_tm_g_bivariate_Setting_encoding_inputs_produces_outputs_without_validation_errors.
shinytest2-tm_g_distribution 💔 $0.08$ $+25.68$ e2e_tm_g_distribution_Histogram_encoding_inputs_produce_output_without_validation_errors.
shinytest2-tm_g_distribution 💔 $0.08$ $+79.00$ e2e_tm_g_distribution_Module_is_initialised_with_the_specified_defaults.
shinytest2-tm_g_distribution 💔 $0.07$ $+16.35$ e2e_tm_g_distribution_QQ_plot_encoding_inputs_produce_output_without_validation_errors.
shinytest2-tm_g_response 💔 $0.08$ $+10.57$ e2e_tm_g_response_deselecting_response_produces_validation_error.
shinytest2-tm_g_response 💔 $0.08$ $+10.46$ e2e_tm_g_response_deselecting_x_produces_validation_error.
shinytest2-tm_g_response 💔 $0.09$ $+19.65$ e2e_tm_g_response_encoding_inputs_produce_output_without_validation_errors.
shinytest2-tm_g_response 💔 $0.09$ $+10.14$ e2e_tm_g_response_module_is_initialised_with_the_specified_defaults.
shinytest2-tm_g_scatterplot 💔 $0.10$ $+13.95$ e2e_tm_g_scatterplot_Base_for_the_log_transformation_can_be_applied.
shinytest2-tm_g_scatterplot 💔 $0.09$ $+12.37$ e2e_tm_g_scatterplot_Get_validation_error_when_facetting_with_the_same_row_col_variable.
shinytest2-tm_g_scatterplot 💔 $0.09$ $+10.62$ e2e_tm_g_scatterplot_Module_is_initialised_with_the_specified_defaults.
shinytest2-tm_g_scatterplot 💔 $0.09$ $+91.09$ e2e_tm_g_scatterplot_The_encoding_inputs_are_set_without_validation_errors.
shinytest2-tm_g_scatterplot 💔 $0.09$ $+14.37$ e2e_tm_g_scatterplot_The_log_transform_is_only_possible_for_positive_numeric_vars.
shinytest2-tm_g_scatterplotmatrix 💔 $0.14$ $+20.56$ e2e_tm_g_scatterplotmatrix_Change_plot_settings
shinytest2-tm_g_scatterplotmatrix 💔 $0.15$ $+13.29$ e2e_tm_g_scatterplotmatrix_Initializes_without_errors
shinytest2-tm_g_scatterplotmatrix 💔 $0.14$ $+17.91$ e2e_tm_g_scatterplotmatrix_Verify_default_values_and_settings_data_extracts_for_data_selection
shinytest2-tm_g_scatterplotmatrix 💔 $0.14$ $+15.42$ e2e_tm_g_scatterplotmatrix_Verify_module_displays_data_table
shinytest2-tm_missing_data 💔 $0.21$ $+19.82$ e2e_tm_missing_data_Check_default_settings_and_visibility_of_the_combinations_graph_and_encodings
shinytest2-tm_missing_data 💔 $0.21$ $+19.70$ e2e_tm_missing_data_Default_settings_and_visibility_of_the_summary_graph
shinytest2-tm_missing_data 💔 $0.20$ $+10.61$ e2e_tm_missing_data_Initializes_without_errors
shinytest2-tm_missing_data 💔 $0.20$ $+19.18$ e2e_tm_missing_data_Validate_functionality_and_UI_response_for_By_Variable_Levels_
shinytest2-tm_outliers 💔 $0.10$ $+16.75$ e2e_tm_outliers_Data_extract_spec_elements_are_initialized_with_the_default_values_specified_by_outlier_var_and_categorical_var_argument.
shinytest2-tm_outliers 💔 $0.10$ $+12.34$ e2e_tm_outliers_Data_parameter_and_module_label_is_passed_properly.
shinytest2-tm_outliers 💔 $0.10$ $+16.35$ e2e_tm_outliers_Default_radio_buttons_are_set_properly.
shinytest2-tm_outliers 💔 $0.09$ $+12.03$ e2e_tm_outliers_Method_parameters_are_set_properly.
shinytest2-tm_outliers 💔 $0.09$ $+12.11$ e2e_tm_outliers_Module_is_divided_into_3_tabs.
shinytest2-tm_outliers 💔 $0.10$ $+34.26$ e2e_tm_outliers_Outlier_definition_text_and_range_are_displayed_properly_depending_on_method.
shinytest2-tm_outliers 💔 $0.10$ $+16.40$ e2e_tm_outliers_Outlier_table_is_displayed_with_proper_content.
shinytest2-tm_outliers 💔 $0.09$ $+18.44$ e2e_tm_outliers_Outliers_summary_table_is_displayed_with_proper_content.
shinytest2-tm_outliers 💔 $0.10$ $+14.36$ e2e_tm_outliers_Plot_type_is_correctly_set_by_default_and_has_appropriate_possible_options.
shinytest2-tm_outliers 💔 $0.09$ $+23.48$ e2e_tm_outliers_Plot_type_is_hidden_when_Boxplot_tab_is_not_selected.
shinytest2-tm_t_crosstable 💔 $0.12$ $+15.33$ e2e_tm_t_crosstable_Change_plot_settings
shinytest2-tm_t_crosstable 💔 $0.39$ $+9.81$ e2e_tm_t_crosstable_Initializes_without_errors
shinytest2-tm_t_crosstable 💔 $0.12$ $+12.94$ e2e_tm_t_crosstable_Verify_default_values_and_settings_data_extracts_for_data_selection
shinytest2-tm_t_crosstable 💔 $0.12$ $+12.32$ e2e_tm_t_crosstable_Verify_module_displays_data_table
shinytest2-tm_variable_browser 💔 $0.10$ $+14.88$ e2e_tm_variable_browser_Selecting_treat_variable_as_factor_changes_the_table_headers.
shinytest2-tm_variable_browser 💔 $0.10$ $+16.67$ e2e_tm_variable_browser_changing_display_density_encoding_doesn_t_show_errors.
shinytest2-tm_variable_browser 💔 $0.11$ $+20.11$ e2e_tm_variable_browser_changing_outlier_definition_encoding_doesn_t_show_errors.
shinytest2-tm_variable_browser 💔 $0.10$ $+22.33$ e2e_tm_variable_browser_changing_plot_setting_encodings_doesn_t_show_errors.
shinytest2-tm_variable_browser 💔 $0.10$ $+13.17$ e2e_tm_variable_browser_content_is_displayed_correctly.
shinytest2-tm_variable_browser 💔 $0.10$ $+12.82$ e2e_tm_variable_browser_selection_of_categorical_variable_has_a_table_with_level_header.
tm_gtsummary 👶 $+0.00$ tm_gtsummary_input_validation_accepts_valid_decorators
tm_gtsummary 👶 $+0.01$ tm_gtsummary_input_validation_creates_a_teal_module_object_with_list_of_data_extract_specs
tm_gtsummary 👶 $+0.01$ tm_gtsummary_input_validation_fails_when_by_allows_multiple_selection
tm_gtsummary 👶 $+0.01$ tm_gtsummary_input_validation_fails_when_by_is_not_a_data_extract_spec_or_list
tm_gtsummary 👶 $+0.01$ tm_gtsummary_input_validation_fails_when_col_label_is_not_character
tm_gtsummary 👶 $+0.01$ tm_gtsummary_input_validation_fails_when_decorators_has_invalid_object_types
tm_gtsummary 👶 $+0.01$ tm_gtsummary_input_validation_fails_when_decorators_is_to_a_different_object
tm_gtsummary 👶 $+0.01$ tm_gtsummary_input_validation_fails_when_include_is_not_a_data_extract_spec_or_list
tm_gtsummary 👶 $+0.01$ tm_gtsummary_input_validation_fails_when_label_is_not_a_string
tm_gtsummary 👶 $+0.00$ tm_gtsummary_input_validation_pass_when_include_allows_multiple_selection
tm_gtsummary 👶 $+0.00$ tm_gtsummary_module_creation_accepts_crane_tbl_roche_summary_arguments
tm_gtsummary 👶 $+0.01$ tm_gtsummary_module_creation_creates_a_module_that_is_bookmarkable
tm_gtsummary 👶 $+0.01$ tm_gtsummary_module_creation_creates_a_module_with_datanames_taken_from_data_extracts
tm_gtsummary 👶 $+0.03$ tm_gtsummary_module_creation_creates_a_teal_module_object
tm_gtsummary 👶 $+0.01$ tm_gtsummary_module_creation_creates_a_teal_module_object_with_list_of_data_extract_specs
tm_gtsummary 👶 $+2.07$ tm_gtsummary_module_server_behavior_server_function_executes_successfully_through_module_interface
tm_gtsummary 👶 $+1.89$ tm_gtsummary_module_server_behavior_server_function_generates_table_with_col_label_
tm_gtsummary 👶 $+0.78$ tm_gtsummary_module_server_behavior_server_function_generates_table_with_include_being_NULL
tm_gtsummary 👶 $+3.94$ tm_gtsummary_module_server_behavior_with_decorators_one_decorator_executes_successfully
tm_gtsummary 👶 $+0.05$ tm_gtsummary_module_ui_behavior_returns_a_htmltools_tag_or_taglist_with_minimal_arguments
tm_outliers 💔 $6.64$ $+1.31$ tm_outliers_module_server_behavior_server_function_handles_different_ggtheme_options_through_module_interface

Results for commit 55370d3

♻️ This comment has been updated with latest results.

@llrs-roche llrs-roche left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just two high levels comments

Comment thread R/tm_gtsummary.R Outdated
Comment thread R/tm_gt_template.R Outdated
tagList(!!!cs_ui),
partial_ui,
# Allow multiple decorators for a single object (table)
teal::ui_transform_teal_data(ns("decorator"), transformators = select_decorators(decorators, "table")),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This might be a "name conflict" but for SME team and other internal users a table is different from a listing.
I am not sure if there are some interest from app-developers on having all transformers as argument so that they only have to copy and paste the module and change the specific part of the gt-related function they want.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ok, I kept the naming from the previous. I'll change this to listing

@averissimo averissimo Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Should tmc modules have been using this in the tm_l_** convention for listings? too late now though.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, the templates id use L instead of T:

But if this was done differently I'm sure there were good reasons or maybe this pattern was not established yet.

Note that I'm saying that depending on what the function used the decorator might be for a table or for a listing. I'm not sure what I used previously or what can be more helpful now.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changed all to listing, please check it out. Should we rename the name of the modules as well?

Or did you just mean the tbl_listing? if that's the case then I'd argue against it as it break from standard :-\

@donyunardi

donyunardi commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Hi @averissimo, thanks for your work on this!

I really like where this is going!

First, we have tbl_gt_template is the generic engine that can process gt/gtsummary object and can pass any function as an object.

Then, we have three functions that covers the most common functions using tbl_gt_template as the engine.

  1. tm_gtsummary -> for gtsummary::tbl_summary
  2. tm_roche_summary -> for crane::tbl_roche_summary
  3. tm_tbl_listing -> for crane::tbl_listing

For these three functions, since they're dedicated to a specific function, I think the .fun argument should be removed and not exposed. I think you eluded to this in your original post as well.

Also, is it possible to use teal.picks for this? Now that we introduce teal.picks, it make sense to make any new module only teal.picks compatible.

Comment thread R/tm_tbl_listing.R Outdated
Comment thread R/tm_gtsummary.R Outdated
Comment thread R/tm_roche_summary.R Outdated
Comment thread R/tm_roche_summary.R Outdated
averissimo and others added 2 commits July 9, 2026 14:31
Co-authored-by: Dony Unardi <donyunardi@gmail.com>
Signed-off-by: André Veríssimo <211358+averissimo@users.noreply.github.com>
Co-authored-by: Dony Unardi <donyunardi@gmail.com>
Signed-off-by: André Veríssimo <211358+averissimo@users.noreply.github.com>
@averissimo

Copy link
Copy Markdown
Contributor Author

@donyunardi

For these three functions, since they're dedicated to a specific function, I think the .fun argument should be removed and not exposed. I think you eluded to this in your original post as well.

I think it's best, the only upside is to allow reuse of same modules if parameters are similar.

Also, is it possible to use teal.picks for this? Now that we introduce teal.picks, it make sense to make any new module only teal.picks compatible.

I was reworking this today and will push it after this round of reviews :-)

@llrs-roche

Copy link
Copy Markdown
Contributor

Then, we have three functions that covers the most common functions using tbl_gt_template as the engine.

  1. tm_gtsummary -> for gtsummary::tbl_summary
  2. tm_roche_summary -> for crane::tbl_roche_summary
  3. tm_tbl_listing -> for crane::tbl_listing

@donyunardi and @averissimo, note that some might be interested in some other functions for example crane::tbl_baseline_chg().

I would keep the flexibility to include other functions. Even if basic functions work reproducing the work of some functions on crane will be hard. I think we should try to be open to support them. Probably shouldn't support all of them from the start, but we should keep it flexible enough for them.

@averissimo
averissimo changed the base branch from main to redesign_extraction@main July 13, 2026 17:24
@averissimo

Copy link
Copy Markdown
Contributor Author

@donyunardi @llrs-roche Updated to picks and incorporated all feedback from PR and discussions.

@donyunardi

Copy link
Copy Markdown
Contributor

Thanks for the update, Andre! Couple comments:

  1. I don't see filter panel for Roche Summary and GT Summary modules. Why is that?
image
  1. Is it possible to choose multiple datasets for by and include argument in tm_tbl_roche_summary?
    When I tried to do this, it doesn't seem to like it:
    tm_tbl_roche_summary(
      label = "Roche Summary",
      by = teal.picks::picks(
        datasets(c("ADSL","ADAE"), "ADSL")
      ),
      include = teal.picks::picks(
        datasets(c("ADSL","ADAE"), "ADSL")
      )
    )

The error message that I got is:

Error in teal.picks::is_pick_multiple(by$variables) : 
  Assertion on 'x' failed: Must inherit from class 'pick', but has class 'NULL'.
  1. in tmg and tmc, we talked about how we want to simplify the module design so that user can just call the module function without any argument and just work. Is it possible to apply the same design here?

@llrs-roche

Copy link
Copy Markdown
Contributor

In addition to Dony's comment, when I try the module tm_tbl_summary() I don't see any UI to change the input:

Captura de pantalla 2026-07-14 094743

@averissimo

Copy link
Copy Markdown
Contributor Author

Thanks for all the feedback team, a bunch of changes happened today and the feature is much more robust.

@donyunardi

  1. I don't see filter panel for Roche Summary and GT Summary modules. Why is that?

Filter panel now appears on all modules.

  1. Is it possible to choose multiple datasets for by and include argument in tm_tbl_roche_summary?
    When I tried to do this, it doesn't seem to like it:

That picks is badly formed as it doesn't have a variables(), but it revealed 2 things:

  1. No check for presence of variables() inside the picks in arguments
  2. Single dataset support was forced

Both were addressed in the commits today, first with some extra checks and latter with datasets being merged using picks.

See updated example with ADSL and ADTTE

  1. in tmg and tmc, we talked about how we want to simplify the module design so that user can just call the module function without any argument and just work. Is it possible to apply the same design here?

Yes, with the exception of tm_tbl_listing that requires a dataname to indicate which dataset to use

@llrs-roche

image

@llrs-roche llrs-roche self-assigned this Jul 14, 2026
Comment thread R/tm_tbl_roche_summary.R
by = NULL,
include = teal.picks::picks(
teal.picks::datasets(),
teal.picks::variables(selected = dplyr::everything(), multiple = TRUE)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

everything() might be too much, would it make sense to select just a single column as default?

The first run with everything is very slow, as well as the default UI doesn't show "Select all / deselect all".

Latter problem can be resolved, but I'm worried about it taking too long to run

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It is the default for gtsummary, so I think this is what users would expect. Note that on github there have been some improvements to speed up gtsummary so the demo could be run with that branch too 😉

@llrs-roche

Copy link
Copy Markdown
Contributor

I see the improvements on the UI, I think now it looks great.
As a low hanging fruit maybe we could add decorators so that modifications on the output could be done easily in the demo.

@averissimo

averissimo commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Decorators are implemented :-) Please feel free to test it out!

Note: It works fine with decorators, I've updated the example to have one of them.

@donyunardi

Copy link
Copy Markdown
Contributor

Thanks for the update! Looks so good!

Couple findings:

  1. Do you see the same thing for table listing?
image
  1. For the drop-down, do we not have the button to select / deselect all? I forgot if we did this teal.picks or not. If we didn't, maybe we should add it as an enhancement.
image

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.

Rename tm_gtsummary to tm_tbl_roche_summary

3 participants