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

Show only datasets used #1309

Merged
merged 9 commits into from
Jan 16, 2025
Merged

Show only datasets used #1309

merged 9 commits into from
Jan 16, 2025

Conversation

llrs-roche
Copy link
Contributor

@llrs-roche llrs-roche commented Jan 9, 2025

Pull Request

Fixes #1307

Only tm_g_ci and tm_g_barcart_simple didn't have the datanames of the module configurable.
I modified tm_g_ci and tm_g_barcart_simple so that only the datasets used are displayed on the module.

See the examples below where the b dataset is not shown on this PR.

`tm_g_ci` example
devtools::load_all("../teal.modules.clinical")
data <- teal_data()
data <- within(data, {
  ADSL <- tmc_ex_adsl
  ADLB <- tmc_ex_adlb
  
  # Note that this dataset when using this module from this PR does not appear.
  b <- data.frame(r = runif(50))
})
join_keys(data) <- default_cdisc_join_keys[names(data)]

ADSL <- data[["ADSL"]]
ADLB <- data[["ADLB"]]

app <- init(
  data = data,
  modules = modules(
    tm_g_ci(
      label = "Confidence Interval Plot",
      x_var = data_extract_spec(
        dataname = "ADSL",
        select = select_spec(
          choices = c("ARMCD", "BMRKR2"),
          selected = c("ARMCD"),
          multiple = FALSE,
          fixed = FALSE
        )
      ),
      y_var = data_extract_spec(
        dataname = "ADLB",
        filter = list(
          filter_spec(
            vars = "PARAMCD",
            choices = levels(ADLB$PARAMCD),
            selected = levels(ADLB$PARAMCD)[1],
            multiple = FALSE,
            label = "Select lab:"
          ),
          filter_spec(
            vars = "AVISIT",
            choices = levels(ADLB$AVISIT),
            selected = levels(ADLB$AVISIT)[1],
            multiple = FALSE,
            label = "Select visit:"
          )
        ),
        select = select_spec(
          label = "Analyzed Value",
          choices = c("AVAL", "CHG"),
          selected = "AVAL",
          multiple = FALSE,
          fixed = FALSE
        )
      ),
      color = data_extract_spec(
        dataname = "ADSL",
        select = select_spec(
          label = "Color by variable",
          choices = c("SEX", "STRATA1", "STRATA2"),
          selected = c("STRATA1"),
          multiple = FALSE,
          fixed = FALSE
        )
      )
    )
  )
)
if (interactive()) {
  shinyApp(app$ui, app$server)
}
`tm_g_barchart_simple` example
devtools::load_all("../teal.modules.clinical")
library(nestcolor)
library(dplyr)

data <- teal_data()
data <- within(data, {
  ADSL <- tmc_ex_adsl %>%
    mutate(ITTFL = factor("Y") %>%
             with_label("Intent-To-Treat Population Flag"))
  ADAE <- tmc_ex_adae %>%
    filter(!((AETOXGR == 1) & (AESEV == "MILD") & (ARM == "A: Drug X")))
  
  # Note that this dataset does not appear on this PR when using this module.
  b <- data.frame(r = runif(50))
})
join_keys(data) <- default_cdisc_join_keys[names(data)]

ADSL <- data[["ADSL"]]
ADAE <- data[["ADAE"]]

app <- init(
  data = data,
  modules = modules(
    tm_g_barchart_simple(
      label = "ADAE Analysis",
      x = data_extract_spec(
        dataname = "ADSL",
        select = select_spec(
          choices = variable_choices(
            ADSL,
            c(
              "ARM", "ACTARM", "SEX",
              "RACE", "ITTFL", "SAFFL", "STRATA2"
            )
          ),
          selected = "ACTARM",
          multiple = FALSE
        )
      ),
      fill = list(
        data_extract_spec(
          dataname = "ADSL",
          select = select_spec(
            choices = variable_choices(
              ADSL,
              c(
                "ARM", "ACTARM", "SEX",
                "RACE", "ITTFL", "SAFFL", "STRATA2"
              )
            ),
            selected = "SEX",
            multiple = FALSE
          )
        ),
        data_extract_spec(
          dataname = "ADAE",
          select = select_spec(
            choices = variable_choices(ADAE, c("AETOXGR", "AESEV", "AESER")),
            selected = NULL,
            multiple = FALSE
          )
        )
      ),
      x_facet = list(
        data_extract_spec(
          dataname = "ADAE",
          select = select_spec(
            choices = variable_choices(ADAE, c("AETOXGR", "AESEV", "AESER")),
            selected = "AETOXGR",
            multiple = FALSE
          )
        ),
        data_extract_spec(
          dataname = "ADSL",
          select = select_spec(
            choices = variable_choices(
              ADSL,
              c(
                "ARM", "ACTARM", "SEX",
                "RACE", "ITTFL", "SAFFL", "STRATA2"
              )
            ),
            selected = NULL,
            multiple = FALSE
          )
        )
      ),
      y_facet = list(
        data_extract_spec(
          dataname = "ADAE",
          select = select_spec(
            choices = variable_choices(ADAE, c("AETOXGR", "AESEV", "AESER")),
            selected = "AESEV",
            multiple = FALSE
          )
        ),
        data_extract_spec(
          dataname = "ADSL",
          select = select_spec(
            choices = variable_choices(
              ADSL,
              c(
                "ARM", "ACTARM", "SEX",
                "RACE", "ITTFL", "SAFFL", "STRATA2"
              )
            ),
            selected = NULL,
            multiple = FALSE
          )
        )
      )
    )
  )
)
if (interactive()) {
  shinyApp(app$ui, app$server)
}

I don't think the documentation of these modules needs to be updated. (The only change on the documentation is to fix a typo)

@llrs-roche llrs-roche added the core label Jan 9, 2025
R/tm_g_ci.R Show resolved Hide resolved
@llrs-roche llrs-roche mentioned this pull request Jan 9, 2025
@m7pr
Copy link
Contributor

m7pr commented Jan 14, 2025

@llrs-roche R CMD CHECKs are failiing. Can you review?

teal.modules.clinical.Rproj Outdated Show resolved Hide resolved
@m7pr m7pr self-assigned this Jan 14, 2025
@llrs-roche
Copy link
Contributor Author

Triggering the tests again, as they were mainly dependency setup, then I'll try to fix them locally

Copy link
Contributor

github-actions bot commented Jan 14, 2025

Unit Tests Summary

    1 files     70 suites   1h 10m 16s ⏱️
  727 tests   614 ✅ 113 💤 0 ❌
1 987 runs  1 759 ✅ 228 💤 0 ❌

Results for commit 0d62e91.

♻️ This comment has been updated with latest results.

@m7pr
Copy link
Contributor

m7pr commented Jan 14, 2025

Stuff like that is visible in the dependency installation for documentation build

Creating lockfile '.github/pkg.lock'Creating lockfile '.github/pkg.lock' [6.2s]
  
  Error: 
  ! error in pak subprocess
  Caused by error: 
  ! Could not solve package dependencies:
  * deps::.: dependency conflict
  * any::sessioninfo: dependency conflict
  ---
  Backtrace:
  1. pak::lockfile_create(c(deps, extra_deps), lockfile = ".github/pkg.lock", …
  2. pak:::remote(function(...) { …
  3. err$throw(res$error)
  ---
  Subprocess backtrace:
  1. base::withCallingHandlers(cli_message = function(msg) { …
  2. get("lockfile_create_internal", asNamespace("pak"))(...)
  3. prop$stop_for_solution_error()
  4. private$plan$stop_for_solve_error()
  5. pkgdepends:::pkgplan_stop_for_solve_error(self, private)
  6. base::throw(new_error("Could not solve package dependencies:\n", msg, …
  7. | base::signalCondition(cond)
  8. global (function (e) …
  Execution halted
  Error: Process completed with exit code 1.

@llrs-roche
Copy link
Contributor Author

I'll compare the Docs/ Pkgdown docs dependency problems with the check run, that could solve the dependencies: https://github.com/insightsengineering/teal.modules.clinical/actions/runs/12687811698/job/35577032936?pr=1309

Copy link
Contributor

github-actions bot commented Jan 14, 2025

Unit Test Performance Difference

Test suite performance difference
Test Suite $Status$ Time on main $±Time$ $±Tests$ $±Skipped$ $±Failures$ $±Errors$
shinytest2-tm_a_gee 💔 $127.28$ $+4.18$ $0$ $0$ $0$ $0$
shinytest2-tm_a_mmrm 💔 $739.41$ $+14.22$ $0$ $0$ $0$ $0$
shinytest2-tm_g_barchart_simple 💔 $223.12$ $+2.48$ $0$ $0$ $0$ $0$
shinytest2-tm_g_forest_rsp 💚 $177.71$ $-1.83$ $0$ $0$ $0$ $0$
shinytest2-tm_g_km 💚 $272.22$ $-2.05$ $0$ $0$ $0$ $0$
shinytest2-tm_g_pp_adverse_events 💔 $124.62$ $+2.38$ $0$ $0$ $0$ $0$
shinytest2-tm_g_pp_patient_timeline 💔 $242.21$ $+1.85$ $0$ $0$ $0$ $0$
shinytest2-tm_g_pp_therapy 💔 $197.98$ $+1.01$ $0$ $0$ $0$ $0$
shinytest2-tm_g_pp_vitals 💔 $85.89$ $+2.31$ $0$ $0$ $0$ $0$
shinytest2-tm_t_abnormality_by_worst_grade 💔 $66.69$ $+1.73$ $0$ $0$ $0$ $0$
shinytest2-tm_t_ancova 💔 $93.66$ $+3.46$ $0$ $0$ $0$ $0$
shinytest2-tm_t_binary_outcome 💔 $75.58$ $+2.19$ $0$ $0$ $0$ $0$
shinytest2-tm_t_coxreg 💔 $72.25$ $+1.71$ $0$ $0$ $0$ $0$
shinytest2-tm_t_events 💔 $60.39$ $+1.74$ $0$ $0$ $0$ $0$
shinytest2-tm_t_events_by_grade 💔 $89.13$ $+2.93$ $0$ $0$ $0$ $0$
shinytest2-tm_t_events_patyear 💔 $50.54$ $+1.09$ $0$ $0$ $0$ $0$
shinytest2-tm_t_events_summary 💔 $66.61$ $+1.09$ $0$ $0$ $0$ $0$
shinytest2-tm_t_exposure 💔 $78.17$ $+1.35$ $0$ $0$ $0$ $0$
shinytest2-tm_t_pp_laboratory 💔 $122.92$ $+2.09$ $0$ $0$ $0$ $0$
shinytest2-tm_t_pp_medical_history 💔 $64.68$ $+1.67$ $0$ $0$ $0$ $0$
shinytest2-tm_t_pp_prior_medication 💔 $76.08$ $+3.48$ $0$ $0$ $0$ $0$
shinytest2-tm_t_shift_by_arm 💔 $57.10$ $+1.97$ $0$ $0$ $0$ $0$
shinytest2-tm_t_shift_by_arm_by_worst 💔 $88.41$ $+1.68$ $0$ $0$ $0$ $0$
shinytest2-tm_t_smq 💔 $57.89$ $+1.02$ $0$ $0$ $0$ $0$
shinytest2-tm_t_summary_by 💔 $76.90$ $+1.38$ $0$ $0$ $0$ $0$
shinytest2-tm_t_tte 💔 $65.24$ $+1.42$ $0$ $0$ $0$ $0$
Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
shinytest2-tm_a_mmrm 💔 $48.26$ $+1.09$ e2e_tm_a_mmrm_Validate_output_on_different_selection_on_method_g_mmrm_lsmeans.
shinytest2-tm_a_mmrm 💔 $48.39$ $+1.05$ e2e_tm_a_mmrm_Validate_output_on_different_selection_on_method_t_mmrm_cov.

Results for commit 231d18d

♻️ This comment has been updated with latest results.

@llrs-roche
Copy link
Contributor Author

The the initial lock files are identical. but on the Docs run when the DESCRIPTION file is modified I see:

Modify DESCRIPTION file (development)
  ℹ Loading metadata database
  ✔ Loading metadata database ... done
  
  ! Failed to resolve dependencies.
  Error:
  * local::teal.modules.clinical:
    * Can't install dependency teal (>= 0.15.2.9079) (>= 0.5.0.9015) (>= 0.6.10.9004) (>= 0.5.0.9012) (>= 0.6.0.9015) (>= 0.3.1.9017)
    * Can't install dependency teal.transform (>= 0.5.0.9015) (>= 0.6.10.9004) (>= 0.5.0.9012) (>= 0.6.0.9015) (>= 0.3.1.9017)
    * Can't install dependency rtables (>= 0.6.10.9004) (>= 0.5.0.9012) (>= 0.6.0.9015) (>= 0.3.1.9017)
    * Can't install dependency teal.code (>= 0.5.0.9012) (>= 0.6.0.9015) (>= 0.3.1.9017)
    * Can't install dependency teal.data (>= 0.6.0.9015) (>= 0.3.1.9017)
    * Can't install dependency teal.reporter (>= 0.3.1.9017)
  End of error
  Failed refs: teal, teal.transform, rtables, teal.code, teal.data, and
  teal.reporter.
  ℹ Adding package teal (ref: "insightsengineering/teal") to the Config/Needs/DepsDev field.
  ℹ Adding package teal.transform (ref: "insightsengineering/teal.transform") to the Config/Needs/DepsDev field.
  ℹ Adding package rtables (ref: "insightsengineering/rtables") to the Config/Needs/DepsDev field.
  ℹ Adding package teal.code (ref: "insightsengineering/teal.code") to the Config/Needs/DepsDev field.
  ℹ Adding package teal.data (ref: "insightsengineering/teal.data") to the Config/Needs/DepsDev field.
  ℹ Adding package teal.reporter (ref: "insightsengineering/teal.reporter") to the Config/Needs/DepsDev field.
  ℹ Trying to resolve dependencies again.
  ---
  ! Failed to resolve dependencies.
  Error:
  * local::teal.modules.clinical:
    * Can't install dependency teal (>= 0.15.2.9079)
    * Can't install dependency insightsengineering/teal * insightsengineering/teal: Can't install dependency teal.slice (>= 0.5.1.9015)
  End of error
  Failed refs: teal, insightsengineering/teal, and teal.slice.
  ℹ Reference for package teal already exists in the Config/Needs/DepsDev field. Skipping.
  ℹ Reference for package teal already exists in the Config/Needs/DepsDev field. Skipping.
  ℹ Adding package teal.slice (ref: "insightsengineering/teal.slice") to the Config/Needs/DepsDev field.
  ℹ Trying to resolve dependencies again.
  ---
  ! Failed to resolve dependencies.
  Error:
  * local::teal.modules.clinical: dependency conflict
  End of error
  Failed refs: .
  ℹ Trying to resolve dependencies again.
  ---
  ! Failed to resolve dependencies.
  Error:
  * local::teal.modules.clinical: dependency conflict
  End of error
  Failed refs: .
  ✖ Failed to resolve dependencies. No progress. Exiting.

teal.slice: main at 0.5.1.9019
teal: main at 0.15.2.9098
So it should be able to find the right version, no sure what is happening here

@llrs-roche
Copy link
Contributor Author

@pawelru there seems to be a problem on the Docs/Pkgdown github actions. The action is failing when the required versions are available on the main branch of their repositories (and it continues beyond the specific Modify DESCRIPTION)

@pawelru
Copy link
Contributor

pawelru commented Jan 14, 2025

This is quite strange

I'm comparing the outcome of check and docs:

check logs

https://github.com/insightsengineering/teal.modules.clinical/actions/runs/12687811698/job/35577032936#step:18:2334

Modify DESCRIPTION file (development)
  ℹ Loading metadata database
  ✔ Loading metadata database ... done
  
  ! Failed to resolve dependencies.
  Error:
  * local::teal.modules.clinical/:
    * Can't install dependency teal (>= 0.15.2.9079) (>= 0.5.0.9015) (>= 0.5.0.9012) (>= 0.6.0.9015) (>= 0.3.1.9017)
    * Can't install dependency teal.transform (>= 0.5.0.9015) (>= 0.5.0.9012) (>= 0.6.0.9015) (>= 0.3.1.9017)
    * Can't install dependency teal.code (>= 0.5.0.9012) (>= 0.6.0.9015) (>= 0.3.1.9017)
    * Can't install dependency teal.data (>= 0.6.0.9015) (>= 0.3.1.9017)
    * Can't install dependency teal.reporter (>= 0.3.1.9017)
  End of error
  Failed refs: teal, teal.transform, teal.code, teal.data, and teal.reporter.
  ℹ Adding package teal (ref: "insightsengineering/teal") to the Config/Needs/DepsDev field.
  ℹ Adding package teal.transform (ref: "insightsengineering/teal.transform") to the Config/Needs/DepsDev field.
  ℹ Adding package teal.code (ref: "insightsengineering/teal.code") to the Config/Needs/DepsDev field.
  ℹ Adding package teal.data (ref: "insightsengineering/teal.data") to the Config/Needs/DepsDev field.
  ℹ Adding package teal.reporter (ref: "insightsengineering/teal.reporter") to the Config/Needs/DepsDev field.
  ℹ Trying to resolve dependencies again.
  ---
  ! Failed to resolve dependencies.
  Error:
  * local::teal.modules.clinical/:
    * Can't install dependency teal (>= 0.15.2.9079)
    * Can't install dependency insightsengineering/teal * insightsengineering/teal: Can't install dependency teal.slice (>= 0.5.1.9015)
  End of error
  Failed refs: teal, insightsengineering/teal, and teal.slice.
  ℹ Reference for package teal already exists in the Config/Needs/DepsDev field. Skipping.
  ℹ Reference for package teal already exists in the Config/Needs/DepsDev field. Skipping.
  ℹ Adding package teal.slice (ref: "insightsengineering/teal.slice") to the Config/Needs/DepsDev field.
  ℹ Trying to resolve dependencies again.
  ---
  ✔ Dependencies resolved successfully.
docs logs

https://github.com/insightsengineering/teal.modules.clinical/actions/runs/12687811632/job/35363019866#step:11:2195

Modify DESCRIPTION file (development)
  ℹ Loading metadata database
  ✔ Loading metadata database ... done
  
  ! Failed to resolve dependencies.
  Error:
  * local::teal.modules.clinical:
    * Can't install dependency teal (>= 0.15.2.9079) (>= 0.5.0.9015) (>= 0.6.10.9004) (>= 0.5.0.9012) (>= 0.6.0.9015) (>= 0.3.1.9017)
    * Can't install dependency teal.transform (>= 0.5.0.9015) (>= 0.6.10.9004) (>= 0.5.0.9012) (>= 0.6.0.9015) (>= 0.3.1.9017)
    * Can't install dependency rtables (>= 0.6.10.9004) (>= 0.5.0.9012) (>= 0.6.0.9015) (>= 0.3.1.9017)
    * Can't install dependency teal.code (>= 0.5.0.9012) (>= 0.6.0.9015) (>= 0.3.1.9017)
    * Can't install dependency teal.data (>= 0.6.0.9015) (>= 0.3.1.9017)
    * Can't install dependency teal.reporter (>= 0.3.1.9017)
  End of error
  Failed refs: teal, teal.transform, rtables, teal.code, teal.data, and
  teal.reporter.
  ℹ Adding package teal (ref: "insightsengineering/teal") to the Config/Needs/DepsDev field.
  ℹ Adding package teal.transform (ref: "insightsengineering/teal.transform") to the Config/Needs/DepsDev field.
  ℹ Adding package rtables (ref: "insightsengineering/rtables") to the Config/Needs/DepsDev field.
  ℹ Adding package teal.code (ref: "insightsengineering/teal.code") to the Config/Needs/DepsDev field.
  ℹ Adding package teal.data (ref: "insightsengineering/teal.data") to the Config/Needs/DepsDev field.
  ℹ Adding package teal.reporter (ref: "insightsengineering/teal.reporter") to the Config/Needs/DepsDev field.
  ℹ Trying to resolve dependencies again.
  ---
  ! Failed to resolve dependencies.
  Error:
  * local::teal.modules.clinical:
    * Can't install dependency teal (>= 0.15.2.9079)
    * Can't install dependency insightsengineering/teal * insightsengineering/teal: Can't install dependency teal.slice (>= 0.5.1.9015)
  End of error
  Failed refs: teal, insightsengineering/teal, and teal.slice.
  ℹ Reference for package teal already exists in the Config/Needs/DepsDev field. Skipping.
  ℹ Reference for package teal already exists in the Config/Needs/DepsDev field. Skipping.
  ℹ Adding package teal.slice (ref: "insightsengineering/teal.slice") to the Config/Needs/DepsDev field.
  ℹ Trying to resolve dependencies again.
  ---
  ! Failed to resolve dependencies.
  Error:
  * local::teal.modules.clinical: dependency conflict
  End of error
  Failed refs: .
  ℹ Trying to resolve dependencies again.
  ---
  ! Failed to resolve dependencies.
  Error:
  * local::teal.modules.clinical: dependency conflict
  End of error
  Failed refs: .
  ✖ Failed to resolve dependencies. No progress. Exiting.

Note that the first two steps are identical. In the first step we do teal, teal.transform, rtables, teal.code, teal.data, and teal.reporter. In the second step we do teal.slice. The third step in check is pass and in docs it is a fail without a meaningful error message. I don't know out of my head what could be a reason. I will have a deeper look tomorrow.

@pawelru
Copy link
Contributor

pawelru commented Jan 15, 2025

I have rerun this and it just works ¯_(ツ)_/¯. Don't ask me why

https://github.com/insightsengineering/teal.modules.clinical/actions/runs/12687811632

@llrs
Copy link

llrs commented Jan 15, 2025

@m7pr I removed the Rstudio changes on Rproj, but see the explanation about its implications and behavior.

@m7pr
Copy link
Contributor

m7pr commented Jan 16, 2025

Just tested examples and non-specified datanames (b) are not shown

image image

@llrs-roche llrs-roche merged commit 42135a1 into main Jan 16, 2025
28 of 29 checks passed
@llrs-roche llrs-roche deleted the 1307_flexible_datanames@main branch January 16, 2025 10:06
@github-actions github-actions bot locked and limited conversation to collaborators Jan 16, 2025
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.

Ability to set datanames in module's constructor
4 participants