Skip to content

Simpify design of tm_g_events_term_id, fix failing CI fixes#343

Merged
osenan merged 18 commits into
redesign_extraction@mainfrom
fix-checks@redesign_extraction@main
Jul 16, 2026
Merged

Simpify design of tm_g_events_term_id, fix failing CI fixes#343
osenan merged 18 commits into
redesign_extraction@mainfrom
fix-checks@redesign_extraction@main

Conversation

@osenan

@osenan osenan commented Jul 14, 2026

Copy link
Copy Markdown

Pull Request

This is part of #330

This is replacing PR #341. The module tm_g_term_id was already on main. Altough the implementation worked, in my opinion it had too complex code which was not required. I have tried to simplify the migration starting from scratch. Then I also made modifications on input validation and use the within.qenv method to have code easier to read.

In this same PR I tried to fix all the errors on CI. Hopefully when this will be merged we can merge it with the other PRs and have clean checks.

The module can be tested with this example:

devtools::load_all()
 data <- teal_data() %>%
   within({
     ADSL <- rADSL
     ADAE <- rADAE
   })

 join_keys(data) <- default_cdisc_join_keys[names(data)]

 app <- init(
   data = data,
   modules = modules(
     tm_g_events_term_id(
       label = "Common AE",
       dataname = "ADAE",
       term_var = variables(
         choices = c(
           "AEDECOD", "AETERM",
           "AEHLT", "AELLT", "AEBODSYS"
         ),
         selected = "AEDECOD"
       ),
       arm_var = variables(
         choices = c("ACTARM", "ACTARMCD"),
         selected = "ACTARMCD"
       ),
       plot_height = c(600, 200, 2000)
     )
   )
 )
 if (interactive()) {
   shinyApp(app$ui, app$server)
 }

``
I found that changing control and treatment values, as changing variables many times does not produce a plot. In addition, the input validation is not very informative. Unfortunately, and due to time restriction. I could not find a pattern that allowed me to improve the input validation.

I checked that in main there is the same behavior related with treatment/control input selection and generation or not of plots. Is something that I will create an issue to improve it.

@osenan
osenan requested a review from a team July 14, 2026 11:39
@osenan osenan added the core label Jul 14, 2026
@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Unit Tests Summary

 1 files   8 suites   1s ⏱️
28 tests 22 ✅ 5 💤 1 ❌
81 runs  74 ✅ 5 💤 2 ❌

For more details on these failures, see this check.

Results for commit aadc831.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Unit Test Performance Difference

Test Suite $Status$ Time on main $±Time$ $±Tests$ $±Skipped$ $±Failures$ $±Errors$
shinytest2-tm_g_events_term_id 👶 $+0.17$ $+5$ $+5$ $0$ $0$
tm_g_ae_oview 👶 $+0.16$ $+9$ $0$ $0$ $0$
tm_g_ae_sub 👶 $+0.08$ $+5$ $0$ $0$ $0$
tm_g_butterfly 👶 $+0.20$ $+11$ $0$ $0$ $0$
tm_g_events_term_id 👶 $+0.06$ $+9$ $0$ $0$ $0$
tm_g_spiderplot 👶 $+0.32$ $+15$ $0$ $+2$ $0$
tm_g_swimlane_waterfall_picks 👶 $+0.12$ $+14$ $0$ $0$ $0$
Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
shinytest2-tm_g_events_term_id 👶 $+0.07$ e2e_tm_g_events_term_id_changing_term_var_changes_the_plot_and_does_not_throw_validation_errors.
shinytest2-tm_g_events_term_id 👶 $+0.01$ e2e_tm_g_events_term_id_deselection_of_term_var_throws_validation_error.
shinytest2-tm_g_events_term_id 👶 $+0.03$ e2e_tm_g_events_term_id_initializes_and_renders_a_plot
shinytest2-tm_g_events_term_id 👶 $+0.01$ e2e_tm_g_events_term_id_sort_updates_title
shinytest2-tm_g_events_term_id 👶 $+0.05$ e2e_tm_g_events_term_id_starts_with_expected_label_and_encoding_selections.
tm_g_ae_oview 👶 $+0.04$ tm_g_ae_oview_argument_verification_fails_when_arm_var_is_neither_picks_or_choices_selected
tm_g_ae_oview 👶 $+0.05$ tm_g_ae_oview_argument_verification_fails_when_flag_var_anl_is_neiter_picks_or_choices_selected
tm_g_ae_oview 👶 $+0.06$ tm_g_ae_oview_module_creation_creates_a_teal_module_using_choices_selected
tm_g_ae_oview 👶 $+0.02$ tm_g_ae_oview_module_creation_creates_a_teal_module_using_picks
tm_g_ae_sub 👶 $+0.06$ tm_g_ae_sub_input_validation_plot_arguments_input_validation
tm_g_ae_sub 👶 $+0.01$ tm_g_ae_sub_module_creation_creates_a_teal_module_using_choices_selected
tm_g_ae_sub 👶 $+0.01$ tm_g_ae_sub_module_creation_creates_a_teal_module_using_picks
tm_g_butterfly 👶 $+0.01$ tm_g_butterfly_input_validation_Forcing_Conversion_from_multiple_picks_to_single
tm_g_butterfly 👶 $+0.16$ tm_g_butterfly_input_validation_plot_arguments_input_validation
tm_g_butterfly 👶 $+0.01$ tm_g_butterfly_module_creation_creates_a_teal_module_using_choices_selected
tm_g_butterfly 👶 $+0.01$ tm_g_butterfly_module_creation_creates_a_teal_module_using_picks
tm_g_events_term_id 👶 $+0.04$ tm_g_events_term_id_builds_a_teal_module_with_picks_encodings
tm_g_events_term_id 👶 $+0.03$ tm_g_events_term_id_coerces_multiple_variable_selection
tm_g_spiderplot 👶 $+0.06$ tm_g_spiderplot_argument_verification_Forcing_Conversion_from_multiple_picks_to_single
tm_g_spiderplot 👶 $+0.23$ tm_g_spiderplot_argument_verification_plot_arguments_input_validation
tm_g_spiderplot 👶 $+0.02$ tm_g_spiderplot_module_creation_creates_a_teal_module_using_choices_selected_default_method_
tm_g_spiderplot 👶 $+0.01$ tm_g_spiderplot_module_creation_creates_a_teal_module_using_picks_.pick_method_
tm_g_swimlane_waterfall_picks 👶 $+0.08$ tm_g_swimlane_builds_a_teal_module_with_picks_encodings
tm_g_swimlane_waterfall_picks 👶 $+0.04$ tm_g_waterfall_builds_a_teal_module_with_picks_encodings

Results for commit d7d13c8

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

badge

Code Coverage Summary

Filename                    Stmts    Miss  Cover    Missing
------------------------  -------  ------  -------  ------------------------------------------------------
R/tm_g_ae_oview.R             271     210  22.51%   90-94, 98-102, 169-402
R/tm_g_ae_sub.R               272     238  12.50%   103-379
R/tm_g_butterfly.R            364     320  12.09%   147, 150, 160-161, 200-567
R/tm_g_decorate.R              42      42  0.00%    17-91
R/tm_g_events_term_id.R       304     265  12.83%   119-418
R/tm_g_heat_bygrade.R         293     293  0.00%    136-459
R/tm_g_patient_profile.R      691     691  0.00%    159-913
R/tm_g_spiderplot.R           322     322  0.00%    100-464
R/tm_g_swimlane.R             333     289  13.21%   65, 124-460
R/tm_g_waterfall.R            415     375  9.64%    80-82, 141-570
R/utils_picks.R               140      38  72.86%   10, 15, 39, 50-51, 103-124, 203, 206, 218-222, 250-254
R/utils.R                      64      59  7.81%    31-73, 107-201
R/zzz.R                         3       3  0.00%    4-7
TOTAL                        3514    3145  10.50%

Diff against main

Filename                   Stmts    Miss  Cover
-----------------------  -------  ------  -------
R/tm_g_ae_oview.R            +46     -15  +22.51%
R/tm_g_ae_sub.R              -16     -50  +12.50%
R/tm_g_butterfly.R            -7     -51  +12.09%
R/tm_g_events_term_id.R      +33      -6  +12.83%
R/tm_g_swimlane.R            -12     -56  +13.21%
R/tm_g_waterfall.R           -14     -54  +9.64%
R/utils_picks.R             +140     +38  +72.86%
R/utils.R                    -21     -21  +1.93%
TOTAL                       +149    -215  +10.35%

Results for commit: 512b2cf

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@osenan osenan changed the title Fix checks@redesign extraction@main Simpify design of tm_g_events_term_id, fix failing CI fixes Jul 14, 2026
Comment thread R/tm_g_events_term_id.R Outdated
…action@main

Signed-off-by: Oriol Senan  <35930244+osenan@users.noreply.github.com>
@llrs-roche llrs-roche self-assigned this Jul 14, 2026
Comment thread R/tm_g_events_term_id.R
Comment thread R/tm_g_events_term_id.R Outdated
Comment thread R/tm_g_events_term_id.R Outdated
Comment thread R/tm_g_events_term_id.R Outdated
Comment thread R/tm_g_events_term_id.R Outdated
osenan and others added 5 commits July 14, 2026 15:42
Co-authored-by: Lluís Revilla <185338939+llrs-roche@users.noreply.github.com>
Signed-off-by: Oriol Senan  <35930244+osenan@users.noreply.github.com>
Co-authored-by: Lluís Revilla <185338939+llrs-roche@users.noreply.github.com>
Signed-off-by: Oriol Senan  <35930244+osenan@users.noreply.github.com>
Comment thread R/tm_g_events_term_id.R Outdated
Comment thread R/tm_g_events_term_id.R Outdated

@averissimo averissimo 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.

Should be good to go. We still have some things to do (re-write swimlane and waterfall), but let's merge this

@osenan
osenan merged commit c430c62 into redesign_extraction@main Jul 16, 2026
25 of 28 checks passed
@osenan
osenan deleted the fix-checks@redesign_extraction@main branch July 16, 2026 13:51
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 16, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants