Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modules should show only relevant datanames #821

Closed
gogonzo opened this issue Aug 16, 2023 · 11 comments · Fixed by #828
Closed

Modules should show only relevant datanames #821

gogonzo opened this issue Aug 16, 2023 · 11 comments · Fixed by #828
Assignees
Labels
bug Something isn't working core

Comments

@gogonzo
Copy link
Contributor

gogonzo commented Aug 16, 2023

Related to questions insightsengineering/teal.code#126 #817

Module asks for "all" datanames instead of restricted number

To fix this it's enough to set dataname = c(parentname, dataname) like in other modules.
Note: Some modules might have different way of specifying datanames - seek for answers in other objects available where teal::module is executed.

Acceptance criteria:

  • Apply the changes in all modules
@m7pr
Copy link
Contributor

m7pr commented Aug 16, 2023

Hey @gogonzo, I tried it and the change from datanames = 'all' to datanames = c(parentname, dataname) in tm_g_pp_adverse_events did not resolve the issue with Show R Code presenting a lot more in teal.gallery patient-profile app which I run locally

@gogonzo gogonzo changed the title tm_g_pp_adverse_events should show only relevant datanames Modules should show only relevant datanames Aug 16, 2023
@gogonzo
Copy link
Contributor Author

gogonzo commented Aug 16, 2023

@m7pr I updated issue description. There are more modules which have datanames = "all". I checked on a tm_t_pp_basic_info and it works 👍

@donyunardi
Copy link
Contributor

donyunardi commented Aug 23, 2023

Essentially, the responsibility to decide which dataset(s) should be included in the module lies with the module developer. If we are making changes to tmc modules, do we need SME involvement to determine which datasets should be presented for each module?

@anajens @shajoezhu any opinion?

@anajens
Copy link
Contributor

anajens commented Aug 23, 2023

Hmm I think this issue probably affects only the patient profile modules. Historically, they were one grouped together as one giant module so perhaps that's why there are multiple datasets still shown?

@m7pr
Copy link
Contributor

m7pr commented Aug 23, 2023

This can be changed so that only relevant data is shown. Showing limited, relevant data, should be less confusing for the end user and should be more consistent with other modules.

@m7pr
Copy link
Contributor

m7pr commented Aug 24, 2023

Essentially, the responsibility to decide which dataset(s) should be included in the module lies with the module developer.

@donyunardi but we are not talking about the dataname parameter passed to tm_g_pp_adverse_events but we are talking about hardcoded value of datanames that is used inside tm_g_pp_adverse_events and passed to modules


app developer can not change that, it is a logic on the implementation level

@donyunardi
Copy link
Contributor

donyunardi commented Aug 25, 2023

app developer can not change that, it is a logic on the implementation level

I understand the confusion. To clarify, I am referring to the teal module developer – the individual that creates tm_g_pp_adverse_events code and hardcode the datanames = "all", not the app developer.

The fact that it's hardcoded leads me to believe that this is intentional during creation. Initially, I thought it would be wise to consult/check with the SME who designed most of the tm_* modules in tmc if they agree with this update before CoreDev starts implementing.

@m7pr
Copy link
Contributor

m7pr commented Aug 25, 2023

Yes. I think @anajens already pointed that it is not expected behavior in #817 and I also noticed it is bringing unneeded output in insightsengineering/teal.code#126

@gogonzo
Copy link
Contributor Author

gogonzo commented Sep 4, 2023

@donyunardi In tm_g_pp_adverse_events only specific datasets are used, which is directly set in server_args here.

    server_args = c(
      data_extract_list,
      list(
        dataname = dataname,
        parentname = parentname,
        ...
      )
    )

After quick code analysis datanames = c(dataname, parentname) is enough for this module as data_extract_list is composed of elements bound to a dataname:

  data_extract_list <- list(
    aeterm = `if`(is.null(aeterm), NULL, cs_to_des_select(aeterm, dataname = dataname)),
    tox_grade = `if`(is.null(tox_grade), NULL, cs_to_des_select(tox_grade, dataname = dataname)),
    causality = `if`(is.null(causality), NULL, cs_to_des_select(causality, dataname = dataname)),
    outcome = `if`(is.null(outcome), NULL, cs_to_des_select(outcome, dataname = dataname)),
    action = `if`(is.null(action), NULL, cs_to_des_select(action, dataname = dataname)),
    time = `if`(is.null(time), NULL, cs_to_des_select(time, dataname = dataname)),
    decod = `if`(is.null(decod), NULL, cs_to_des_select(decod, dataname = dataname))
  )

@m7pr
Copy link
Contributor

m7pr commented Sep 4, 2023

@donyunardi would you put it in one of the sprints of this increment?

@donyunardi
Copy link
Contributor

sure, let's tackle this next sprint.

@averissimo averissimo self-assigned this Sep 15, 2023
averissimo added a commit that referenced this issue Sep 19, 2023
# Pull Request

- Fixes #821

### How to test

Use
[`patient-profile`](https://github.com/insightsengineering/teal.gallery/blob/main/patient-profile/app.R)
app from `teal.gallery`, or the code below.

Note: the code below that adds 2 `tm_t_summary` modules to
patient-profile _(the 2nd doesn't use `dataname = "ADSL"` to show that
`ADSL` is added as `parentname`)_

<details>

<summary>See to code</summary>

```R
pkgload::load_all()
library(teal.modules.clinical)
library(teal.modules.general)
library(scda)
library(scda.2022)
library(nestcolor)

options(shiny.useragg = FALSE)

nest_logo <- "https://raw.githubusercontent.com/insightsengineering/hex-stickers/main/PNG/nest.png"

ADSL <- synthetic_cdisc_data("latest")$adsl
ADMH <- synthetic_cdisc_data("latest")$admh
ADAE <- synthetic_cdisc_data("latest")$adae
ADCM <- synthetic_cdisc_data("latest")$adcm
ADVS <- synthetic_cdisc_data("latest")$advs
ADLB <- synthetic_cdisc_data("latest")$adlb

## Modify ADCM
ADCM$CMINDC <- paste0("Indication_", as.numeric(ADCM$CMDECOD))
ADCM$CMDOSE <- 1
ADCM$CMTRT <- ADCM$CMCAT
ADCM$CMDOSU <- "U"
ADCM$CMROUTE <- "CMROUTE"
ADCM$CMDOSFRQ <- "CMDOSFRQ"
ADCM$CMASTDTM <- ADCM$ASTDTM
ADCM$CMAENDTM <- ADCM$AENDTM

teal.data::col_labels(
  ADCM[c("CMINDC", "CMTRT", "ASTDY", "AENDY")]
) <- c(
  "Indication",
  "Reported Name of Drug, Med, or Therapy",
  "Study Day of Start of Medication",
  "Study Day of End of Medication"
)

## Modify ADHM
ADMH[["MHDISTAT"]] <- "ONGOING"
teal.data::col_labels(ADMH[c("MHDISTAT")]) <- c("Status of Disease")

## Define variable inputs
aeterm_input <- data_extract_spec(
  dataname = "ADAE",
  select = select_spec(
    choices = variable_choices(ADAE, "AETERM"),
    selected = c("AETERM"),
    multiple = FALSE,
    fixed = FALSE
  )
)

cmtrt_input <- data_extract_spec(
  dataname = "ADCM",
  select = select_spec(
    choices = variable_choices(ADCM, "CMTRT"),
    selected = c("CMTRT"),
    multiple = FALSE,
    fixed = FALSE
  )
)

cmindc_input <- data_extract_spec(
  dataname = "ADCM",
  select = select_spec(
    choices = variable_choices(ADCM, "CMINDC"),
    selected = c("CMINDC"),
    multiple = FALSE,
    fixed = FALSE
  )
)

atirel_input <- data_extract_spec(
  dataname = "ADCM",
  select = select_spec(
    choices = variable_choices(ADCM, "ATIREL"),
    selected = c("ATIREL"),
    multiple = FALSE,
    fixed = FALSE
  )
)

cmdecod_input <- data_extract_spec(
  dataname = "ADCM",
  select = select_spec(
    choices = variable_choices(ADCM, "CMDECOD"),
    selected = c("CMDECOD"),
    multiple = FALSE,
    fixed = FALSE
  )
)

app <- init(
  data = cdisc_data(
    cdisc_dataset("ADSL", ADSL, code = "ADSL <- synthetic_cdisc_data(\"latest\")$adsl"),
    cdisc_dataset("ADAE", ADAE, code = "ADAE <- synthetic_cdisc_data(\"latest\")$adae"),
    cdisc_dataset("ADMH", ADMH, code = "ADMH <- synthetic_cdisc_data(\"latest\")$admh
      ADMH[['MHDISTAT']] <- 'ONGOING'
      teal.data::col_labels(ADMH[c('MHDISTAT')]) <- c('Status of Disease')"),
    cdisc_dataset("ADCM", ADCM, code = 'ADCM <- synthetic_cdisc_data(\"latest\")$adcm
      ADCM$CMINDC <- paste0("Indication_", as.numeric(ADCM$CMDECOD))
      ADCM$CMDOSE <- 1
      ADCM$CMTRT <- ADCM$CMCAT
      ADCM$CMDOSU <- "U"
      ADCM$CMROUTE <- "CMROUTE"
      ADCM$CMDOSFRQ <- "CMDOSFRQ"
      ADCM$CMASTDTM <- ADCM$ASTDTM
      ADCM$CMAENDTM <- ADCM$AENDTM
      teal.data::col_labels(
        ADCM[c("CMINDC", "CMTRT", "ASTDY", "AENDY")]) <- c(
          "Indication",
          "Reported Name of Drug, Med, or Therapy",
          "Study Day of Start of Medication",
          "Study Day of End of Medication")'),
    cdisc_dataset("ADVS", ADVS, code = "ADVS <- synthetic_cdisc_data(\"latest\")$advs"),
    cdisc_dataset("ADLB", ADLB, code = "ADLB <- synthetic_cdisc_data(\"latest\")$adlb"),
    check = TRUE
  ),
  filter = teal_slices(
    count_type = "all",
    teal_slice(dataname = "ADSL", varname = "SEX"),
    teal_slice(dataname = "ADSL", varname = "AGE")
  ),
  modules = modules(
    tm_front_page(
      label = "App Info",
      header_text = c("Info about input data source" = "This app uses CDISC ADaM datasets randomly generated by `scda` & `scda.2022` R packages"),
      tables = list(`NEST packages used in this demo app` = data.frame(Packages = c("teal.modules.general", "teal.modules.clinical", "scda", "scda.2022")))
    ),
    tm_t_summary(
      label = "Demographic Table",
      dataname = "ADSL",
      arm_var = choices_selected(c("ARM", "ARMCD"), "ARM"),
      add_total = TRUE,
      summarize_vars = choices_selected(
        c("SEX", "RACE", "BMRKR2", "EOSDY", "DCSREAS", "AGE"),
        c("SEX", "RACE")
      ),
      useNA = "ifany"
    ),
    tm_t_summary(
      label = "Demographic Table (2)",
      dataname = "ADMH",
      arm_var = choices_selected(c("ARM", "ARMCD"), "ARM"),
      add_total = TRUE,
      summarize_vars = choices_selected(
        c("SEX", "RACE", "BMRKR2", "EOSDY", "DCSREAS", "AGE"),
        c("SEX", "RACE")
      ),
      useNA = "ifany"
    ),
    tm_t_pp_basic_info(
      label = "Basic info",
      dataname = "ADSL",
      patient_col = "USUBJID",
      vars = data_extract_spec(
        dataname = "ADSL",
        select = select_spec(
          choices = variable_choices(ADSL),
          selected = c("ARM", "AGE", "SEX", "COUNTRY", "RACE", "EOSSTT"),
          multiple = TRUE,
          fixed = FALSE
        )
      )
    ),
    tm_t_pp_medical_history(
      label = "Medical history",
      parentname = "ADSL",
      patient_col = "USUBJID",
      mhterm = data_extract_spec(
        dataname = "ADMH",
        select = select_spec(
          choices = variable_choices(ADMH, c("MHTERM")),
          selected = c("MHTERM"),
          multiple = FALSE,
          fixed = FALSE
        )
      ),
      mhbodsys = data_extract_spec(
        dataname = "ADMH",
        select = select_spec(
          choices = variable_choices(ADMH, "MHBODSYS"),
          selected = c("MHBODSYS"),
          multiple = FALSE,
          fixed = FALSE
        )
      ),
      mhdistat = data_extract_spec(
        dataname = "ADMH",
        select = select_spec(
          choices = variable_choices(ADMH, "MHDISTAT"),
          selected = c("MHDISTAT"),
          multiple = FALSE,
          fixed = FALSE
        )
      )
    ),
    tm_t_pp_prior_medication(
      label = "Prior medication",
      parentname = "ADSL",
      patient_col = "USUBJID",
      atirel = atirel_input,
      cmdecod = cmdecod_input,
      cmindc = cmindc_input,
      cmstdy = data_extract_spec(
        dataname = "ADCM",
        select = select_spec(
          choices = variable_choices(ADCM, "ASTDY"),
          selected = c("ASTDY"),
          multiple = FALSE,
          fixed = FALSE
        )
      )
    ),
    tm_g_pp_vitals(
      label = "Vitals",
      parentname = "ADSL",
      patient_col = "USUBJID",
      plot_height = c(600L, 200L, 2000L),
      paramcd = data_extract_spec(
        dataname = "ADVS",
        select = select_spec(
          choices = variable_choices(ADVS, "PARAMCD"),
          selected = c("PARAMCD"),
          multiple = FALSE,
          fixed = FALSE
        )
      ),
      xaxis = data_extract_spec(
        dataname = "ADVS",
        select = select_spec(
          choices = variable_choices(ADVS, "ADY"),
          selected = c("ADY"),
          multiple = FALSE,
          fixed = FALSE
        )
      ),
      aval = data_extract_spec(
        dataname = "ADVS",
        select = select_spec(
          choices = variable_choices(ADVS, "AVAL"),
          selected = c("AVAL"),
          multiple = FALSE,
          fixed = FALSE
        )
      )
    ),
    tm_g_pp_therapy(
      label = "Therapy",
      parentname = "ADSL",
      patient_col = "USUBJID",
      plot_height = c(600L, 200L, 2000L),
      atirel = atirel_input,
      cmdecod = cmdecod_input,
      cmindc = cmindc_input,
      cmdose = data_extract_spec(
        dataname = "ADCM",
        select = select_spec(
          choices = variable_choices(ADCM, "CMDOSE"),
          selected = c("CMDOSE"),
          multiple = FALSE,
          fixed = FALSE
        )
      ),
      cmtrt = cmtrt_input,
      cmdosu = data_extract_spec(
        dataname = "ADCM",
        select = select_spec(
          choices = variable_choices(ADCM, "CMDOSU"),
          selected = c("CMDOSU"),
          multiple = FALSE,
          fixed = FALSE
        )
      ),
      cmroute = data_extract_spec(
        dataname = "ADCM",
        select = select_spec(
          choices = variable_choices(ADCM, "CMROUTE"),
          selected = c("CMROUTE"),
          multiple = FALSE,
          fixed = FALSE
        )
      ),
      cmdosfrq = data_extract_spec(
        dataname = "ADCM",
        select = select_spec(
          choices = variable_choices(ADCM, "CMDOSFRQ"),
          selected = c("CMDOSFRQ"),
          multiple = FALSE,
          fixed = FALSE
        )
      ),
      cmstdy = data_extract_spec(
        dataname = "ADCM",
        select = select_spec(
          choices = variable_choices(ADCM, "ASTDY"),
          selected = c("ASTDY"),
          multiple = FALSE,
          fixed = FALSE
        )
      ),
      cmendy = data_extract_spec(
        dataname = "ADCM",
        select = select_spec(
          choices = variable_choices(ADCM, "AENDY"),
          selected = c("AENDY"),
          multiple = FALSE,
          fixed = FALSE
        )
      )
    ),
    tm_g_pp_adverse_events(
      label = "Adverse events",
      parentname = "ADSL",
      patient_col = "USUBJID",
      plot_height = c(600L, 200L, 2000L),
      aeterm = aeterm_input,
      tox_grade = data_extract_spec(
        dataname = "ADAE",
        select = select_spec(
          choices = variable_choices(ADAE, "AETOXGR"),
          selected = c("AETOXGR"),
          multiple = FALSE,
          fixed = FALSE
        )
      ),
      causality = data_extract_spec(
        dataname = "ADAE",
        select = select_spec(
          choices = variable_choices(ADAE, "AEREL"),
          selected = c("AEREL"),
          multiple = FALSE,
          fixed = FALSE
        )
      ),
      outcome = data_extract_spec(
        dataname = "ADAE",
        select = select_spec(
          choices = variable_choices(ADAE, "AEOUT"),
          selected = c("AEOUT"),
          multiple = FALSE,
          fixed = FALSE
        )
      ),
      action = data_extract_spec(
        dataname = "ADAE",
        select = select_spec(
          choices = variable_choices(ADAE, "AEACN"),
          selected = c("AEACN"),
          multiple = FALSE,
          fixed = FALSE
        )
      ),
      time = data_extract_spec(
        dataname = "ADAE",
        select = select_spec(
          choices = variable_choices(ADAE, "ASTDY"),
          selected = c("ASTDY"),
          multiple = FALSE,
          fixed = FALSE
        )
      ),
      decod = NULL
    ),
    tm_t_pp_laboratory(
      label = "Lab values",
      parentname = "ADSL",
      patient_col = "USUBJID",
      paramcd = data_extract_spec(
        dataname = "ADLB",
        select = select_spec(
          choices = variable_choices(ADLB, "PARAMCD"),
          selected = c("PARAMCD"),
          multiple = FALSE,
          fixed = FALSE
        )
      ),
      param = data_extract_spec(
        dataname = "ADLB",
        select = select_spec(
          choices = variable_choices(ADLB, "PARAM"),
          selected = c("PARAM"),
          multiple = FALSE,
          fixed = FALSE
        )
      ),
      timepoints = data_extract_spec(
        dataname = "ADLB",
        select = select_spec(
          choices = variable_choices(ADLB, "ADY"),
          selected = c("ADY"),
          multiple = FALSE,
          fixed = FALSE
        )
      ),
      anrind = data_extract_spec(
        dataname = "ADLB",
        select = select_spec(
          choices = variable_choices(ADLB, "ANRIND"),
          selected = c("ANRIND"),
          multiple = FALSE,
          fixed = FALSE
        )
      ),
      aval = data_extract_spec(
        dataname = "ADLB",
        select = select_spec(
          choices = variable_choices(ADLB, "AVAL"),
          selected = c("AVAL"),
          multiple = FALSE,
          fixed = FALSE
        )
      ),
      avalu = data_extract_spec(
        dataname = "ADLB",
        select = select_spec(
          choices = variable_choices(ADLB, "AVALU"),
          selected = c("AVALU"),
          multiple = FALSE,
          fixed = FALSE
        )
      )
    ),
    tm_g_pp_patient_timeline(
      label = "Patient timeline",
      parentname = "ADSL",
      patient_col = "USUBJID",
      plot_height = c(600L, 200L, 2000L),
      font_size = c(15L, 8L, 25L),
      cmdecod = cmdecod_input,
      aeterm = aeterm_input,
      aetime_start = data_extract_spec(
        dataname = "ADAE",
        select = select_spec(
          choices = variable_choices(ADAE, "ASTDTM"),
          selected = c("ASTDTM"),
          multiple = FALSE,
          fixed = FALSE
        )
      ),
      aetime_end = data_extract_spec(
        dataname = "ADAE",
        select = select_spec(
          choices = variable_choices(ADAE, "AENDTM"),
          selected = c("AENDTM"),
          multiple = FALSE,
          fixed = FALSE
        )
      ),
      dstime_start = data_extract_spec(
        dataname = "ADCM",
        select = select_spec(
          choices = variable_choices(ADCM, "CMASTDTM"),
          selected = c("CMASTDTM"),
          multiple = FALSE,
          fixed = FALSE
        )
      ),
      dstime_end = data_extract_spec(
        dataname = "ADCM",
        select = select_spec(
          choices = variable_choices(ADCM, "CMAENDTM"),
          selected = c("CMAENDTM"),
          multiple = FALSE,
          fixed = FALSE
        )
      ),
      aerelday_start = data_extract_spec(
        dataname = "ADAE",
        select = select_spec(
          choices = variable_choices(ADAE, "ASTDY"),
          selected = c("ASTDY"),
          multiple = FALSE,
          fixed = FALSE
        )
      ),
      aerelday_end = data_extract_spec(
        dataname = "ADAE",
        select = select_spec(
          choices = variable_choices(ADAE, "AENDY"),
          selected = c("AENDY"),
          multiple = FALSE,
          fixed = FALSE
        )
      ),
      dsrelday_start = data_extract_spec(
        dataname = "ADCM",
        select = select_spec(
          choices = variable_choices(ADCM, "ASTDY"),
          selected = c("ASTDY"),
          multiple = FALSE,
          fixed = FALSE
        )
      ),
      dsrelday_end = data_extract_spec(
        dataname = "ADCM",
        select = select_spec(
          choices = variable_choices(ADCM, "AENDY"),
          selected = c("AENDY"),
          multiple = FALSE,
          fixed = FALSE
        )
      )
    )
  ),
  header = tags$span(
    style = "display: flex; align-items: center; justify-content: space-between; margin: 10px 0 10px 0;",
    tags$head(tags$link(rel = "shortcut icon", href = nest_logo), tags$title("Patient Profile Analysis Teal Demo App")),
    tags$span(
      style = "font-size: 30px;",
      "Example teal app focusing on patient-level analysis of clinical trial data with teal.modules.clinical"
    ),
    tags$span(
      style = "display: flex; align-items: center;",
      tags$img(src = nest_logo, alt = "NEST logo", height = "45px", style = "margin-right:10px;"),
      tags$span(style = "font-size: 24px;", "NEST @ Roche")
    )
  ),
  footer = tags$p(
    actionLink("showAboutModal", "About,"),
    tags$a(
      href = "https://github.com/insightsengineering/teal.gallery/tree/main/patient-profile",
      target = "_blank",
      "Source Code,"
    ),
    tags$a(
      href = "https://github.com/insightsengineering/teal.gallery/issues",
      target = "_blank",
      "Report Issues"
    )
  )
)

body(app$server)[[length(body(app$server)) + 1]] <- quote(
  observeEvent(input$showAboutModal, {
    showModal(modalDialog(
      tags$p("This teal app is brought to you by the NEST Team at Roche/Genentech. For more information, please visit:"),
      tags$ul(
        tags$li(tags$a(
          href = "https://github.com/insightsengineering", "Insights Engineering",
          target = "blank"
        )),
        tags$li(tags$a(
          href = "https://pharmaverse.org", "Pharmaverse",
          target = "blank"
        ))
      ),
      easyClose = TRUE
    ))
  })
)

shinyApp(app$ui, app$server)
```

</details>

### Changes description

With the exception of `tm_t_summary`, only patient_profile modules were
modified.

- Change to `datanames = c(dataname)`
  - `tm_t_pp_basic_info`
  
- Change to `datanames = c(dataname_adcm, dataname_adae, parentname)`
  - `tm_g_pp_patient_timeline`

* Change to `datanames = c(dataname, parentname)`
  * `tm_g_pp_adverse_events`
  * `tm_g_pp_therapy`
  * `tm_g_pp_vitals`
  * `tm_t_pp_laboratory`
  * `tm_t_pp_medical_history`
  * `tm_t_pp_prior_medication`
  * `tm_t_summary`
* Special case, as `parentname` server parameter is used on
`data_extract_list`, but was not used in `module()` call
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working core
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants