WIP: PoC, gtsummary on tm_t_events_summary#1420
Conversation
Unit Tests Summary 1 files 71 suites 42s ⏱️ Results for commit ea43658. ♻️ This comment has been updated with latest results. |
Unit Test Performance DifferenceAdditional test case details
Results for commit b1351f3 ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Opening this comment to prevent merging as this module should replace tm_t_events_summary once it is ready.
…eering/teal.modules.clinical into tm_t_events_gtsummary@main
|
Hi @llrs-roche I don't see any table result when trying your example code: @kumamiao also got the same behavior. Do you experience the same thing? SessionInfor$> sessionInfo()
R version 4.5.1 (2025-06-13)
Platform: aarch64-apple-darwin20
Running under: macOS Sequoia 15.6.1
Matrix products: default
BLAS: /System/Library/Frameworks/Accelerate.framework/Versions/A/Frameworks/vecLib.framework/Versions/A/libBLAS.dylib
LAPACK: /Library/Frameworks/R.framework/Versions/4.5-arm64/Resources/lib/libRlapack.dylib; LAPACK version 3.12.1
locale:
[1] en_US.UTF-8/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8
time zone: America/Los_Angeles
tzcode source: internal
attached base packages:
[1] stats graphics grDevices utils datasets methods base
other attached packages:
[1] teal.modules.clinical_0.11.1.9002 teal.widgets_0.5.0.9000 gtsummary_2.4.0 dplyr_1.1.4
[5] tern_0.9.9.9001 rtables_0.6.13.9000 magrittr_2.0.3 formatters_0.5.11.9002
[9] teal.transform_0.7.0.9000 teal_1.0.0.9003 teal.slice_0.7.0.9000 teal.data_0.8.0.9000
[13] teal.code_0.7.0.9001 shiny_1.11.1 testthat_3.2.3 multiloadr_0.0.2
loaded via a namespace (and not attached):
[1] Rdpack_2.6.4 mmrm_0.3.15.9001 tern.mmrm_0.3.3.9000 bsicons_0.1.2 formatR_1.14 remotes_2.5.0
[7] tern.gee_0.1.5.9008 logger_0.4.0 sandwich_3.1-1 rlang_1.1.6 multcomp_1.4-28 compiler_4.5.1
[13] vctrs_0.6.5 profvis_0.4.0 pkgconfig_2.0.3 fastmap_1.2.0 backports_1.5.0 ellipsis_0.3.2
[19] fontawesome_0.5.3 promises_1.3.2 rmarkdown_2.29 sessioninfo_1.2.3 purrr_1.0.4 xfun_0.52
[25] shinyvalidate_0.1.3 cachem_1.1.0 teal.reporter_0.5.0.9001 jsonlite_2.0.0 later_1.4.2 styler_1.10.3
[31] parallel_4.5.1 broom_1.0.8 R6_2.6.1 bslib_0.9.0 stringi_1.8.7 RColorBrewer_1.1-3
[37] parallelly_1.44.0 pkgload_1.4.0 brio_1.1.5 jquerylib_0.1.4 estimability_1.5.1 assertthat_0.2.1
[43] Rcpp_1.0.14 knitr_1.50 usethis_3.1.0 zoo_1.8-14 teal.logger_0.4.0.9001 R.utils_2.13.0
[49] httpuv_1.6.16 Matrix_1.7-3 splines_4.5.1 R.cache_0.17.0 tidyselect_1.2.1 rstudioapi_0.17.1
[55] TMB_1.9.17 codetools_0.2-20 miniUI_0.1.2 rlistings_0.2.12.9001 pkgbuild_1.4.8 lattice_0.22-7
[61] tibble_3.2.1 withr_3.0.2 evaluate_1.0.3 desc_1.4.3 survival_3.8-3 urlchecker_1.0.1
[67] xml2_1.3.8 shinycssloaders_1.1.0 pillar_1.10.2 checkmate_2.3.2 DT_0.33 shinyjs_2.1.0
[73] plotly_4.10.4 generics_0.1.4 rprojroot_2.0.4 ggplot2_3.5.2 scales_1.4.0 xtable_1.8-4
[79] glue_1.8.0 nestcolor_0.1.3 lazyeval_0.2.2 emmeans_1.11.1 tools_4.5.1 data.table_1.17.4
[85] fs_1.6.6 mvtnorm_1.3-3 cowplot_1.1.3 grid_4.5.1 tidyr_1.3.1 rbibutils_2.3
[91] devtools_2.4.5 nlme_3.1-168 cli_3.6.5 vistime_1.2.4 viridisLite_0.4.2 gt_1.0.0
[97] geepack_1.3.12 gtable_0.3.6 R.methodsS3_1.8.2 sass_0.4.10 digest_0.6.37 ggrepel_0.9.6
[103] TH.data_1.1-3 htmlwidgets_1.6.4 farver_2.1.2 memoise_2.0.1 htmltools_0.5.8.1 R.oo_1.27.1
[109] lifecycle_1.0.4 httr_1.4.7 shinyWidgets_0.9.0 mime_0.13 MASS_7.3-65 |
|
Apologies @donyunardi and @kumamiao, I forgot to push one small commit to teal.widgets. |
donyunardi
left a comment
There was a problem hiding this comment.
Thanks for prepping this @llrs-roche
My personal take on this is that right now the function is very tightly coupled, lots of hardcoded stuff in the server which makes it hard to modified.
However, I wonder if this is what we want or need from business standpoint.
I personally think what you have right now might be enough to show the tlg team to get some guidance on how flexible these setup should be.
| bslib::accordion_panel( | ||
| "Table Settings", | ||
| open = TRUE, | ||
| checkboxInput( | ||
| ns("count_dth"), | ||
| "Count deaths", | ||
| value = a$count_dth | ||
| ), | ||
| checkboxInput( | ||
| ns("count_wd"), | ||
| "Count withdrawals due to AE", | ||
| value = a$count_wd | ||
| ), | ||
| checkboxInput( | ||
| ns("count_subj"), | ||
| "Count patients", | ||
| value = a$count_subj | ||
| ), | ||
| checkboxInput( | ||
| ns("count_pt"), | ||
| "Count preferred terms", | ||
| value = a$count_pt | ||
| ), | ||
| checkboxInput( | ||
| ns("count_events"), | ||
| "Count events", | ||
| value = a$count_events | ||
| ) | ||
| ), |
There was a problem hiding this comment.
Should this be wrap around bslib::accordion()? Currently, I don't see these inputs.
There was a problem hiding this comment.
I didn't modify much the UI. This is how it is on tm_t_events_summary: https://github.com/insightsengineering/teal.modules.clinical/blob/main/R/tm_t_events_summary.R#L793.
But I noticed some weird issues with UI elements like the table not showing up here with the widget, but when I try the widget alone it works well. I'll explore this
| labels <- list( | ||
| # Those that must be (DTHFL and AEWITHFL are given more descriptive titles) | ||
| ae_any = "Total number of participants with at least one AE", | ||
| ae_count = "Total number of AEs", | ||
| DTHFL = "Total number of deaths", | ||
| AEWITHFL = "Total number of participants withdrawn from study due to an AE", | ||
| # Those that are calculated | ||
| ae_death = "AE with fatal outcome", | ||
| ae_serious = "Serious AE", | ||
| ae_ser_withdraw = "Serious AE leading to withdrawal from treatment", | ||
| ae_ser_acn = "Serious AE leading to dose modification/interruption", | ||
| ae_sae_rel = "Related Serious AE", | ||
| ae_withdraw = "AE leading to withdrawal from treatment", | ||
| ae_acn = "AE leading to modification/interruption", | ||
| ae_rel = "Related AE", | ||
| ae_rel_withdraw = "Related AE leading to withdrawal from treatment", | ||
| ae_rel_acn = "Related AE leading to dose modication/interruption", | ||
| ae_sev = "Severe AE (at greatest intensity)" | ||
| ) |
There was a problem hiding this comment.
Should label be something that we surface as the function's argument rather than hardcoding in the server function? This way, if we change how this is worded, we can just update the label argument.
There was a problem hiding this comment.
I didn't saw that exposed on tm_t_events_summary. There are many rows/labels and I think they need to be consistent across tables and datasets so I hardcoded them. The modules as is lacks some features that would make it more flexible. In that case, I append the new labels to these hardcoded. If we exposed this the user should match the input specified with the labels, which could create more troubles for the users.
| library("gtsummary") | ||
| library("dplyr") | ||
| library("crane") | ||
| selection_AEACN <- c("DRUG INTERRUPTED", "DOSE INCREASED", "DOSE REDUCED") |
There was a problem hiding this comment.
I don't think we should hardcode this.
There was a problem hiding this comment.
I wasn't sure if this is common to ADaM datasets or it should be flexible. It looked like it was fixed so I hardcoded it. If it changes we could expose a new input to select those values (with the new teal.transform this should be easier).
| dunlin::subject_level_flag( | ||
| data_long = ADAE, | ||
| # Any AE | ||
| ae_any = TRUE, | ||
| # Serious AE leading to withdrawal from treatment | ||
| ae_ser_withdraw = AESER == "Y" & AEACN == "DRUG WITHDRAWN", | ||
| # Serious AE leading to dose modification/interruption | ||
| ae_ser_acn = AESER == "Y" & AEACN %in% selection_AEACN, | ||
| # Related Serious AE | ||
| ae_sae_rel = AESER == "Y" & AEREL == "Y", | ||
| # AE Leading to withdrawal from treatment | ||
| ae_withdraw = AEACN == "DRUG WITHDRAWN", | ||
| # AE leading to modification/interruption | ||
| ae_acn = AEACN %in% selection_AEACN, | ||
| # Related AE | ||
| ae_rel = AEREL == "Y", | ||
| # Related AE leading to withdrawal from treatment | ||
| ae_rel_withdraw = AEREL == "Y" & AEACN == "DRUG WITHDRAWN", | ||
| # Related AE leading to dose modification/interruption | ||
| ae_rel_acn = AEREL == "Y" & AEACN %in% selection_AEACN, | ||
| # Severe AE (at greatest intensity) | ||
| ae_sev = AESEV == "SEVERE", | ||
| # AE with fatal outcome | ||
| ae_death = AESDTH == "Y", | ||
| # Serious AE | ||
| ae_serious = AESER == "Y" |
There was a problem hiding this comment.
I understand why this is here, because we're trying to match the processing steps in the catalog. But, I feel that this can be done during teal_data object creation.
If we put this outside of server, and together with the label, user will have more control on these steps.
There was a problem hiding this comment.
I agree, users will have more control if this is not hardcoded here. But at the same time this will increase the validation efforts of the input data (that is logical and one row per patient/the row size of ADSL).
There was a problem hiding this comment.
Module Flexibility and tm_t_events_gtsummary
Thanks for the the feedback! The current implementation of tm_t_events_summary might be a bit too rigid because I followed the examples very closely. I'm not sure if the columns used in those examples are meant to be fixed, or if users should be able to customize them. I'm ready to adjust this based on guidance.
While we want to avoid making the module so flexible that it dilutes its resembles tm_t_crosstable, I believe we can add the full functionality of the original tm_t_events_summary without compromising this principle. The existing module allows users to select from a predefined set of variables, which is a key feature. To add this, we'll need to handle the merging of different tables based on user input, which will make the module more dynamic and user-friendly (but I understood this wasn't part of this PoC).
Improving Data Manipulation in teal.modules.clinical
On a broader note, we're using a long chain of dplyr functions to join tables and create output tables. This makes our current approach to manipulating ADaM datasets is long and flexible but not focused or taking advantage of the properties of the characteristics of the dataset(s).
I propose we standardize our dataset manipulation by using a specialized package designed for ADaM datasets. Packages like dunlin, admiral, or dm are built to handle the complexities of multiple related tables. By adopting a tool like this, I wish we could summarize data with a single function call flexible enough that we could add new columns or summaries depending on input choices. This would be a vast improvement over a long series of dplyr calls.
This change would make our modules more maintainable, less error-prone, more flexible and at the same time more focused to ADaM datasets. The table formatting packages like gtsummary or rtables shouldn't try to mix the data manipulation from the table presentation. I believe gtsummary does a great job at this. But this shift on data manipulation on modules would be a significant step forward for the entire teal.modules.clinical package too.
| bslib::accordion_panel( | ||
| "Table Settings", | ||
| open = TRUE, | ||
| checkboxInput( | ||
| ns("count_dth"), | ||
| "Count deaths", | ||
| value = a$count_dth | ||
| ), | ||
| checkboxInput( | ||
| ns("count_wd"), | ||
| "Count withdrawals due to AE", | ||
| value = a$count_wd | ||
| ), | ||
| checkboxInput( | ||
| ns("count_subj"), | ||
| "Count patients", | ||
| value = a$count_subj | ||
| ), | ||
| checkboxInput( | ||
| ns("count_pt"), | ||
| "Count preferred terms", | ||
| value = a$count_pt | ||
| ), | ||
| checkboxInput( | ||
| ns("count_events"), | ||
| "Count events", | ||
| value = a$count_events | ||
| ) | ||
| ), |
There was a problem hiding this comment.
I didn't modify much the UI. This is how it is on tm_t_events_summary: https://github.com/insightsengineering/teal.modules.clinical/blob/main/R/tm_t_events_summary.R#L793.
But I noticed some weird issues with UI elements like the table not showing up here with the widget, but when I try the widget alone it works well. I'll explore this
| library("gtsummary") | ||
| library("dplyr") | ||
| library("crane") | ||
| selection_AEACN <- c("DRUG INTERRUPTED", "DOSE INCREASED", "DOSE REDUCED") |
There was a problem hiding this comment.
I wasn't sure if this is common to ADaM datasets or it should be flexible. It looked like it was fixed so I hardcoded it. If it changes we could expose a new input to select those values (with the new teal.transform this should be easier).
| labels <- list( | ||
| # Those that must be (DTHFL and AEWITHFL are given more descriptive titles) | ||
| ae_any = "Total number of participants with at least one AE", | ||
| ae_count = "Total number of AEs", | ||
| DTHFL = "Total number of deaths", | ||
| AEWITHFL = "Total number of participants withdrawn from study due to an AE", | ||
| # Those that are calculated | ||
| ae_death = "AE with fatal outcome", | ||
| ae_serious = "Serious AE", | ||
| ae_ser_withdraw = "Serious AE leading to withdrawal from treatment", | ||
| ae_ser_acn = "Serious AE leading to dose modification/interruption", | ||
| ae_sae_rel = "Related Serious AE", | ||
| ae_withdraw = "AE leading to withdrawal from treatment", | ||
| ae_acn = "AE leading to modification/interruption", | ||
| ae_rel = "Related AE", | ||
| ae_rel_withdraw = "Related AE leading to withdrawal from treatment", | ||
| ae_rel_acn = "Related AE leading to dose modication/interruption", | ||
| ae_sev = "Severe AE (at greatest intensity)" | ||
| ) |
There was a problem hiding this comment.
I didn't saw that exposed on tm_t_events_summary. There are many rows/labels and I think they need to be consistent across tables and datasets so I hardcoded them. The modules as is lacks some features that would make it more flexible. In that case, I append the new labels to these hardcoded. If we exposed this the user should match the input specified with the labels, which could create more troubles for the users.
| dunlin::subject_level_flag( | ||
| data_long = ADAE, | ||
| # Any AE | ||
| ae_any = TRUE, | ||
| # Serious AE leading to withdrawal from treatment | ||
| ae_ser_withdraw = AESER == "Y" & AEACN == "DRUG WITHDRAWN", | ||
| # Serious AE leading to dose modification/interruption | ||
| ae_ser_acn = AESER == "Y" & AEACN %in% selection_AEACN, | ||
| # Related Serious AE | ||
| ae_sae_rel = AESER == "Y" & AEREL == "Y", | ||
| # AE Leading to withdrawal from treatment | ||
| ae_withdraw = AEACN == "DRUG WITHDRAWN", | ||
| # AE leading to modification/interruption | ||
| ae_acn = AEACN %in% selection_AEACN, | ||
| # Related AE | ||
| ae_rel = AEREL == "Y", | ||
| # Related AE leading to withdrawal from treatment | ||
| ae_rel_withdraw = AEREL == "Y" & AEACN == "DRUG WITHDRAWN", | ||
| # Related AE leading to dose modification/interruption | ||
| ae_rel_acn = AEREL == "Y" & AEACN %in% selection_AEACN, | ||
| # Severe AE (at greatest intensity) | ||
| ae_sev = AESEV == "SEVERE", | ||
| # AE with fatal outcome | ||
| ae_death = AESDTH == "Y", | ||
| # Serious AE | ||
| ae_serious = AESER == "Y" |
There was a problem hiding this comment.
I agree, users will have more control if this is not hardcoded here. But at the same time this will increase the validation efforts of the input data (that is logical and one row per patient/the row size of ADSL).
|
Closing PR because we want to try different approach for the PoC. |

Pull Request
Fixes https://github.com/insightsengineering/coredev-tasks/issues/658
Proof of concept of how gtsummary could work with on modules creating tables.
This PR: introduces a new "module"
tm_t_events_gtsummarythat istm_t_events_summarytweaked for using gtsummary.It also adds a new dependency dunlin that helps with merging ADSL and ADAE, so that we (teal.*) are not responsible of handling this as much as we did previously.
We can get similar visual output.
To explore:
Showing common rows and grouped flags is not part of the PoC (according to comments elsewhere). But I explored it and it is a matter of summarizing the tables to be a one row per USUBJID and then adding it to the data.frame/tibble that is going to be used for the creation of the table.
Making it more flexible and less hard-coded maybe by exploring how to interact with {dunlin} functions better.
The function used
subject_level_flagis very new (v 0.1.11 from 2025-08-22).It doesn't handle specifying new conditions from the ADAE and ADSL: teal modules will still need to manage that.
To pass new conditions not hard-coded on
teal_dataone can usedplyr::exprbut the column name is fixed: Depending on the level of flexibility needed by the different variations of table it could be easier to modify the code directly:Adding a new variable
When switching between the variables on
arm_var, there is an error because the input processing isn't delayed: one needs to deselect all, wait for validation to kick in and then select the new variable so that theteal_dataobject is correctly processed.The numbers of "All patients" is different to calculated without gtsummary: the options within
gtsummary::add_overalldon't match those currently calculated in some cases.There is also a bug on
add_overallthat prevents rendering a new line on the column label: I opened an issue there: New line on the column label ofadd_overallisn't rendered ddsjoberg/gtsummary#2310teal.widgets::table_with_settings_srvdoesn't supportgtclass to resize tables and save it (or change orientation)On this PR POC: Checking gtsummary works well with widgets for tables teal.widgets#317 I tested the widgets could be made to work.
Minimal example