Skip to content

[WIP] picks version for tm_g_events_term_id#329

Draft
m7pr wants to merge 1 commit intomainfrom
redesign_extraction@main
Draft

[WIP] picks version for tm_g_events_term_id#329
m7pr wants to merge 1 commit intomainfrom
redesign_extraction@main

Conversation

@m7pr
Copy link
Contributor

@m7pr m7pr commented Mar 5, 2026

Based on

and a new package

Changes

Refactored tm_g_events_term_id to support the new picks interface from teal.picks, following the same S3 dispatch pattern established in teal.modules.general PR #942 (based on tm_g_response).

tm_g_events_term_id.R

  • Converted tm_g_events_term_id() into an S3 generic dispatching on term_var
  • Renamed the original implementation to tm_g_events_term_id.default()
  • Updated roxygen docs to document the dispatch behavior

tm_g_events_term_id_picks.R (new)

  • Added tm_g_events_term_id.picks() method using teal.picks::picks() for term_var and arm_var
  • Uses teal.picks::picks_ui() / picks_srv() for variable selection UI
  • Uses teal.picks::merge_srv() for dataset merging with automatic join key handling
  • Includes .picks_datanames() helper for resolving datanames from picks objects

DESCRIPTION

  • Added teal.picks (>= 0.1.0) to Imports
  • Added insightsengineering/teal.picks to Remotes
Tested with this code
devtools::load_all("teal")
devtools::load_all("teal.picks")
devtools::load_all("osprey")
devtools::load_all("teal.osprey")

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

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

app <- init(
  data = data,
  modules = modules(
    # Picks method (new)
    tm_g_events_term_id(
      label = "Common AE (picks)",
      term_var = teal.picks::picks(
        teal.picks::datasets("ADAE"),
        teal.picks::variables(
          choices = teal.picks::is_categorical(min.len = 2),
          selected = "AEDECOD"
        )
      ),
      arm_var = teal.picks::picks(
        teal.picks::datasets("ADSL"),
        teal.picks::variables(
          choices = teal.picks::is_categorical(min.len = 2),
          selected = "ACTARMCD"
        )
      ),
      plot_height = c(600, 200, 2000)
    ),
    # Default method (old) for comparison
    tm_g_events_term_id(
      label = "Common AE (default)",
      dataname = "ADAE",
      term_var = teal.transform::choices_selected(
        selected = "AEDECOD",
        choices = c("AEDECOD", "AETERM", "AEHLT", "AELLT", "AEBODSYS")
      ),
      arm_var = teal.transform::choices_selected(
        selected = "ACTARMCD",
        choices = c("ACTARM", "ACTARMCD")
      ),
      plot_height = c(600, 200, 2000)
    )
  )
)

shinyApp(app$ui, app$server)

Default to teal.transform

defualt

Picks method

picks

TODO

  • fix plot resize
  • add tests

Comment on lines +68 to +81
term_var = teal.picks::picks(
teal.picks::datasets(),
teal.picks::variables(
choices = teal.picks::is_categorical(min.len = 2),
selected = 1L
)
),
arm_var = teal.picks::picks(
teal.picks::datasets(),
teal.picks::variables(
choices = teal.picks::is_categorical(min.len = 2),
selected = 1L
)
),
Copy link
Contributor

@averissimo averissimo Mar 13, 2026

Choose a reason for hiding this comment

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

I think this is a good candidate to use variables() in the arguments, instead of picks

Suggested change
term_var = teal.picks::picks(
teal.picks::datasets(),
teal.picks::variables(
choices = teal.picks::is_categorical(min.len = 2),
selected = 1L
)
),
arm_var = teal.picks::picks(
teal.picks::datasets(),
teal.picks::variables(
choices = teal.picks::is_categorical(min.len = 2),
selected = 1L
)
),
term_var = teal.picks::variables(
choices = teal.picks::is_categorical(min.len = 2),
selected = 1L
),
arm_var = teal.picks::variables(
choices = teal.picks::is_categorical(min.len = 2),
selected = 1L
),

And then generate the actual picks on body

term_var <- picks::picks(teal.picks::datasets(dataname), term_var)
arm_var <- picks::picks(teal.picks::datasets(dataname), arm_var)

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not make that transparent to the user?

term_var = teal.picks::picks(datasets(dataname), variables(....),
arm_var = teal.picks::picks(datasets(dataname), variables(....),

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.

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

3 participants