Add picks to tm_g_butterfly module by osenan · Pull Request #335 · insightsengineering/teal.osprey

osenan · 2026-03-19T13:10:29Z

It solves part of #330

Following similar strategy to #331 we are only having one picks srv and ui function and converting choices_selected to picks. However, there is a bug under investigation on teal.picks and currently the example crashes if we change the value of any variable on the default method.

Please try this example to test the app

library(teal)
library(teal.picks)
devtools::load_all()

 data <- teal_data() %>%
   eval_code("set.seed(23) # @linksto ADSL") %>%
   within({
     library(nestcolor)
     library(dplyr)
     ADSL <- rADSL
     ADAE <- rADAE
     ADSL <- mutate(ADSL, DOSE = paste(sample(1:3, n(), replace = TRUE), "UG"))
     ADAE <- mutate(
       ADAE,
       flag1 = ifelse(AETOXGR == 1, 1, 0),
       flag2 = ifelse(AETOXGR == 2, 1, 0),
       flag3 = ifelse(AETOXGR == 3, 1, 0),
       flag1_filt = rep("Y", n())
     )
   })

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

 app <- init(
   data = data,
   modules = modules(
     tm_g_butterfly(
       label = "Butterfly Plot (picks)",
       dataname = "ADAE",
       right_var = teal.picks::variables(
         choices = c("SEX", "ARM", "RACE"),
         selected = "SEX"
       ),
       left_var = teal.picks::variables(
         choices = c("SEX", "ARM", "RACE"),
         selected = "RACE"
       ),
       category_var = teal.picks::variables(
         choices = c("AEDECOD", "AEBODSYS"),
         selected = "AEBODSYS"
       ),
       color_by_var = teal.picks::variables(
         choices = c("AETOXGR", "None"),
         selected = "AETOXGR"
       ),
       count_by_var = teal.picks::values(
         selected = "# of patients",
         choices = c("# of patients", "# of AEs")
       ),
       facet_var = teal.picks::variables(
         choices = c("RACE", "SEX", "ARM"),
         selected = NULL
       ),
       sort_by_var = teal.picks::values(
         selected = "count",
         choices = c("count", "alphabetical")
       ),
       legend_on = TRUE,
       plot_height = c(600, 200, 2000)
     ),
     tm_g_butterfly(
       label = "Butterfly Plot (default)",
       dataname = "ADAE",
       right_var = choices_selected(
         selected = "SEX",
         choices = c("SEX", "ARM", "RACE")
       ),
       left_var = choices_selected(
         selected = "SEX",
         choices = c("SEX", "ARM", "RACE")
       ),
       category_var = choices_selected(
         selected = "AEBODSYS",
         choices = c("AEDECOD", "AEBODSYS")
       ),
       color_by_var = choices_selected(
         selected = "AETOXGR",
         choices = c("AETOXGR", "None")
       ),
       count_by_var = choices_selected(
         selected = "# of patients",
         choices = c("# of patients", "# of AEs")
       ),
       facet_var = choices_selected(
         selected = NULL,
         choices = c("RACE", "SEX", "ARM")
       ),
       sort_by_var = choices_selected(
         selected = "count",
         choices = c("count", "alphabetical")
       ),
       legend_on = TRUE,
       plot_height = c(600, 200, 2000)
     )
   )
 )
 if (interactive()) {
   shinyApp(app$ui, app$server)
 }

github-actions · 2026-03-19T13:10:43Z

✅ All contributors have signed the CLA
_{Posted by the CLA Assistant Lite bot.}

osenan · 2026-03-19T13:10:57Z

I have read the CLA Document and I hereby sign the CLA

… method

github-actions · 2026-03-20T09:09:56Z

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                    288     288  0.00%    60-379
R/tm_g_butterfly.R                 360     316  12.22%   147, 150, 160-161, 200-561
R/tm_g_decorate.R                   42      42  0.00%    17-91
R/tm_g_events_term_id_picks.R      305     305  0.00%    23-406
R/tm_g_events_term_id.R            289     247  14.53%   131-407
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                  332     288  13.25%   65, 124-459
R/tm_g_waterfall.R                 414     374  9.66%    80-82, 141-569
R/utils_picks.R                    129      43  66.67%   18, 29-30, 70-74, 82-103, 129, 182, 185, 197-201, 223, 229-233
R/utils.R                           64      59  7.81%    31-73, 107-201
R/zzz.R                              3       3  0.00%    4-7
TOTAL                             3803    3481  8.47%

Diff against main

Filename                         Stmts    Miss  Cover
-----------------------------  -------  ------  --------
R/tm_g_ae_oview.R                  +46     -15  +22.51%
R/tm_g_butterfly.R                 -11     -55  +12.22%
R/tm_g_events_term_id_picks.R     +305    +305  +100.00%
R/tm_g_events_term_id.R            +18     -24  +14.53%
R/tm_g_swimlane.R                  -13     -57  +13.25%
R/tm_g_waterfall.R                 -15     -55  +9.66%
R/utils_picks.R                   +129     +43  +66.67%
R/utils.R                          -21     -21  +1.93%
TOTAL                             +438    +121  +8.32%

Results for commit: b9db744

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

github-actions · 2026-03-20T09:10:29Z

Unit Tests Summary

1 files ± 0 6 suites +1 0s ⏱️ ±0s
21 tests + 4 16 ✅ + 4 5 💤 ±0 0 ❌ ±0
60 runs +11 55 ✅ +11 5 💤 ±0 0 ❌ ±0

Results for commit b9db744. ± Comparison against base commit 726b039.

♻️ This comment has been updated with latest results.

github-actions · 2026-03-20T09:10:33Z

Unit Test Performance Difference

Test Suite	$Status$	$±Time$	$±Tests$	$±Skipped$	$±Failures$	$±Errors$
shinytest2-tm_g_events_term_id	👶	$+0.17$	$+5$	$+5$	$0$	$0$
tm_g_ae_oview	👶	$+0.17$	$+9$	$0$	$0$	$0$
tm_g_butterfly	👶	$+0.22$	$+11$	$0$	$0$	$0$
tm_g_events_term_id	👶	$+0.06$	$+8$	$0$	$0$	$0$
tm_g_swimlane_waterfall_picks	👶	$+0.13$	$+14$	$0$	$0$	$0$

Additional test case details

Test Suite	$Status$	$±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_butterfly	👶	$+0.02$	tm_g_butterfly_input_validation_Forcing_Conversion_from_multiple_picks_to_single
tm_g_butterfly	👶	$+0.18$	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_rejects_multiple_variable_selection
tm_g_swimlane_waterfall_picks	👶	$+0.08$	tm_g_swimlane_builds_a_teal_module_with_picks_encodings
tm_g_swimlane_waterfall_picks	👶	$+0.05$	tm_g_waterfall_builds_a_teal_module_with_picks_encodings

Results for commit e7056d7

♻️ This comment has been updated with latest results.

osenan · 2026-06-25T15:06:42Z

I added the following changes:

Use picks_utils to transform choices selected and values
remove S3 dispatch
Do not prefix with teal.picks examples in documentation

I tested the example app manually and I observe no issue in the behavior. The CI fails but is due other modules, not due to code changed in this PR.

osenan · 2026-07-02T13:57:06Z

+        shiny::validate(
+          shiny::need(length(right_var_name) > 0, "Right Dichotomization Variable is required."),
+          shiny::need(length(left_var_name) > 0, "Left Dichotomization Variable is required."),
+          shiny::need(length(category_var_name) > 0, "Category Variable is required."),
+          shiny::need(
+            is.factor(ANL[[right_var_name]]),
+            "Right Dichotomization Variable must be a factor variable, contact developer."
+          ),
+          shiny::need(
+            is.factor(ANL[[left_var_name]]),
+            "Left Dichotomization Variable must be a factor variable, contact developer."


Do you see a problem that it is using shiny::validate instead of teal::validate_input?

llrs-roche

Great work.

I was a bit surprised when running the app and I saw some input elements weren't using picks for selecting values. The "Choose Up To 2:" can be done with an teal::validate_input(), not sure how it has been done on tmc or other packages.

Very few comments about the code itself. I'd like to have more tests but that is out of the scope for now.

llrs-roche · 2026-07-03T11:49:41Z

-#'         selected = "AETOXGR",
-#'         choices = c("AETOXGR", "None")
+#'       color_by_var = variables(
+#'         choices = c("AETOXGR", "None"),


Should None be replaced by allowing no selection? I don't think None is a valid variable on that environment and this doesn't allow to disable the colors.

Yes, good observation. I changed to allow no color in the plot and updated the example

llrs-roche · 2026-07-03T11:54:00Z

+  color_by_var <- migrate_choices_selected_to_variables(color_by_var, multiple = FALSE)
+  filter_var <- migrate_choices_selected_to_variables(filter_var, null.ok = TRUE)
+  facet_var <- migrate_choices_selected_to_variables(facet_var, null.ok = TRUE)
+  count_by_var <- migrate_choices_selected_to_values(count_by_var)


This function returns a pick of class values.

Yes, but I think it is the correct class of the object. It is a pick class of values because it is not connected to datasets or variables, the choices are independent of the dataset.

llrs-roche · 2026-07-03T12:08:05Z

+  if (teal.picks::is_pick_multiple(right_var$variables)) {
+    warning(
+      "`right_var` accepts only a single variable selection. ",
+      "Forcing `teal.picks::variables(multiple)` to FALSE."
+    )
+    attr(right_var$variables, "multiple") <- FALSE
+  }


I think this is also a good candidate for a helper function. Note that is repeated 4 times in only this module. Something like

as_pick_single <- function(var, name) { if (teal.picks::is_pick_multiple(right_var$variables)) { warning( sprintf("`%s` accepts only a single variable selection. ", name) "Forcing `teal.picks::variables(multiple)` to FALSE." ) attr(var "multiple") <- FALSE var }

That could be used as right_var$variables <- as_pick_single(right_var$variables, "right_var")

Agree! Changed

llrs-roche · 2026-07-03T12:14:13Z

+        facet_var_name <- merged$variables()$facet_var

-        teal::validate_inputs(iv())
+        teal::validate_has_data(ANL, min_nrow = 0, msg = "ANL Data is empty")


I guess here we are mimicking previous checks. But, wouldn't it make sense to add min_nrow = 10 or some other value? This might be a case of leave the code better than what we found.

I agree with you, but I suggest 1 as minimum for filtering with big constraints

llrs-roche · 2026-07-03T13:03:22Z

+              }),
+              id = ANL$USUBJID,
+              facet_rows = .(if (!is.null(facet_var_name)) {
+                bquote(ANL[[.(facet_var_name)]])


Are those many bquotes needed? Then it might be worth to consider using within

Suggested change

bquote(ANL[[.(facet_var_name)]])

ANL[[.(facet_var_name)]]

Yes, I changed everything to within so it is more readable

osenan · 2026-07-07T11:56:22Z

Hi, many thanks for your suggestions. I tried to apply all of them. Regarding right_val and left_val we could do a simplification of the module. Instead of right_var being a variable, it could be a picks where user chooses variable and values. The UI would be simplified a little bit. Probably it would look better. The only thing that I am not sure if we can implement is to constraint, on the same picks, only the options of the selected values in the UI to be up to two values while the UI of the picks variables be a single selection

osenan added 2 commits March 19, 2026 14:07

chore update NAMESPACE and DESCRIPTION to new dependencies

0f9899e

tests: add tests for tm_g_butterfly module

c50aa06

osenan added the core label Mar 19, 2026

osenan mentioned this pull request Mar 20, 2026

[Feature Request]: Allow modules to use teal.picks #330

Open

osenan added 2 commits March 20, 2026 10:04

refactor: use only picks ui and function for butterfly module default…

8de0ae3

… method

chore: update lintr

e3c479d

osenan marked this pull request as ready for review March 20, 2026 09:05

osenan requested a review from a team March 20, 2026 09:08

This was referenced Mar 26, 2026

Add picks to tm_g_heat_bygrade module #337

Merged

[Feature Request]: define and/or export .picks_datanames insightsengineering/teal.picks#17

Closed

osenan changed the base branch from main to redesign_extraction@main June 23, 2026 13:53

osenan and others added 5 commits June 23, 2026 15:57

Merge branch 'redesign_extraction@main' into tm_g_butterfly_picks@main

c752366

refactor: use utils picks function to create values

2fce115

[skip style] [skip vbump] Restyle files

20cd390

chore: fix style

adadb21

chore: fix R CMD check

8e7aa86

osenan requested a review from averissimo June 25, 2026 15:07

osenan mentioned this pull request Jun 25, 2026

Use picks in tm_g_spiderplot module #336

Merged

docs: avoid calling teal.picks in examples

104e08f

osenan commented Jul 2, 2026

View reviewed changes

refactor: use teal::validate_input instead of shiny::validate/need

81895a7

llrs-roche self-assigned this Jul 3, 2026

llrs-roche reviewed Jul 3, 2026

View reviewed changes

refactor: improve example, plot colouring and data validation

8074bd4

osenan and others added 3 commits July 7, 2026 13:44

refactor: use utils function to simplify forcing to single picks

0f9f6a9

docs: update documentation

4d305ad

[skip style] [skip vbump] Restyle files

e89689a

refactor: use formals to call the ui function

b9db744

osenan mentioned this pull request Jul 8, 2026

[Feature Request]: Improve the UI when using a picks object with values insightsengineering/teal.picks#95

Open

3 tasks

llrs-roche approved these changes Jul 10, 2026

View reviewed changes

Comment thread R/utils_picks.R

osenan merged commit 1557e34 into redesign_extraction@main Jul 13, 2026
24 of 27 checks passed

osenan deleted the tm_g_butterfly_picks@main branch July 13, 2026 04:51

github-actions Bot locked and limited conversation to collaborators Jul 13, 2026

Uh oh!

Uh oh!

Conversation

osenan commented Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

osenan commented Mar 19, 2026

Uh oh!

github-actions Bot commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage Summary

Diff against main

Uh oh!

github-actions Bot commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Unit Tests Summary

Uh oh!

github-actions Bot commented Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Unit Test Performance Difference

Uh oh!

osenan commented Jun 25, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

llrs-roche left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

osenan commented Jul 7, 2026

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

osenan commented Mar 19, 2026 •

edited

Loading

github-actions Bot commented Mar 19, 2026 •

edited

Loading

github-actions Bot commented Mar 20, 2026 •

edited

Loading

github-actions Bot commented Mar 20, 2026 •

edited

Loading

github-actions Bot commented Mar 20, 2026 •

edited

Loading