Migrate tm_g_terms_id to picks

[osenan]

Pull Request

It partially fixes #330

Please try the module 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",
       term_dataname = "ADAE",
       term_var = variables(
         choices = is_categorical(min.len = 2),
         selected = "AEDECOD"
       ),
       arm_dataname = "ADSL",
       arm_var = variables(
         choices = is_categorical(min.len = 2),
         selected = "ACTARMCD"
       ),
       plot_height = c(600, 200, 2000)
     )
   )
 )
 if (interactive()) {
   shinyApp(app$ui, app$server)
 }

This PR is replacing #329

[osenan]

This util needs to be refactored to be consistent with other modules that force the picks to be single

[llrs-roche]

Do you mean that this line needs to change or that there are further changes needed on utils.R or on teal.picks to provide this functionality?

[github-actions]

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               271     237  12.55%   103-377
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.R       315     266  15.56%   140-436
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      41  68.22%   18, 29-30, 70-74, 82-103, 182, 185, 197-201, 229-233
R/utils.R                      64      59  7.81%    31-73, 107-201
R/zzz.R                         3       3  0.00%    4-7
TOTAL                        3507    3142  10.41%

Diff against main

Filename                   Stmts    Miss  Cover
-----------------------  -------  ------  -------
R/tm_g_ae_oview.R            +46     -15  +22.51%
R/tm_g_ae_sub.R              -17     -51  +12.55%
R/tm_g_butterfly.R           -11     -55  +12.22%
R/tm_g_events_term_id.R      +44      -5  +15.56%
R/tm_g_swimlane.R            -13     -57  +13.25%
R/tm_g_waterfall.R           -15     -55  +9.66%
R/utils_picks.R             +129     +41  +68.22%
R/utils.R                    -21     -21  +1.93%
TOTAL                       +142    -218  +10.26%

Results for commit: dbe53a3

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

[github-actions]

Unit Tests Summary

 1 files   7 suites   1s ⏱️
24 tests 19 ✅ 5 💤 0 ❌
65 runs  60 ✅ 5 💤 0 ❌

Results for commit dbe53a3.

♻️ This comment has been updated with latest results.

[github-actions]

Unit Test Performance Difference

Test Suite	$Status$	Time on main	$±Time$	$±Tests$	$±Skipped$	$±Failures$	$±Errors$
shinytest2-tm_g_events_term_id	👶		$+0.19$	$+5$	$+5$	$0$	$0$
tm_g_ae_oview	👶		$+0.17$	$+9$	$0$	$0$	$0$
tm_g_ae_sub	👶		$+0.09$	$+5$	$0$	$0$	$0$
tm_g_butterfly	👶		$+0.22$	$+11$	$0$	$0$	$0$
tm_g_events_term_id	👶		$+0.26$	$+8$	$0$	$0$	$0$
tm_g_swimlane_waterfall_picks	👶		$+0.14$	$+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.06$	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.07$	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.07$	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.02$	tm_g_butterfly_input_validation_Forcing_Conversion_from_multiple_picks_to_single
tm_g_butterfly	👶		$+0.19$	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.14$	tm_g_events_term_id_builds_a_teal_module_with_picks_encodings
tm_g_events_term_id	👶		$+0.12$	tm_g_events_term_id_rejects_multiple_variable_selection
tm_g_swimlane_waterfall_picks	👶		$+0.09$	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 2ce6d02

♻️ This comment has been updated with latest results.

[llrs-roche]

The default input on the example/test case provided generates an uninformative error (see comments below):

I also see that two new arguments were added arm_dataname and term_dataname. Why? teal.picks should be able to handle that, and at most I would expect one dataname needed but not two.

[llrs-roche]

Do you mean that this line needs to change or that there are further changes needed on utils.R or on teal.picks to provide this functionality?

[llrs-roche]

Remember to pass the variables inside the reactive that are not present on the qenv:

Suggested change

	})
	},
	term_var_name = term_var_name,)

You'll need to do the same with other variables that are passed as arguments.Line 418, shouldn't need to use get().

[osenan]

Closed due to this PR #343