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

All datanames to transform@669 insert UI@main #1302

Merged

Conversation

gogonzo
Copy link
Contributor

@gogonzo gogonzo commented Aug 8, 2024

Changed the way how datanames are passed between teal_data instances:

Previously:

  1. loading data
  2. filtering active* datasets + creating new teal_data with datanames(data) <- active_datanames + subsetting code and environment to the active datasets
  3. transforming data
  4. passing filtered and transformed data to the module. Adjusting datanames to the datasets added in teal_transform
  • active means data for currently active module

Now:

  1. loading data
  2. filtering active datasets
  3. transforming data
  4. creating new teal_data with datanames(data) <- active_datanames + subsetting code and environment to the active datasets - passing to the module

Thanks to the new way it is possible now:

  • create a new object in teal_transform_module for a module
  • module$datanames doesn't need to include related datanames in order to execute teal_transform

See the app below on this branch and on the feature branch:

app
options(
  teal.log_level = "TRACE",
  teal.show_js_log = TRUE,
  # teal.bs_theme = bslib::bs_theme(version = 5),
  shiny.bookmarkStore = "server"
)

# pkgload::load_all("teal.data")
pkgload::load_all("teal.slice")
pkgload::load_all("teal")

anl_transformer <- list(
  teal_transform_module(
    label = "ANL",
    ui = function(id) {
      ns <- NS(id)
      tagList(
        div("This transformer adds ANL (ADSL + ADTTE)")
      )
    },
    server = function(id, data) {
      moduleServer(id, function(input, output, session) {
        reactive({
          within(data(), {
            ANL <- dplyr::inner_join(
              ADSL,
              ADTTE[c("STUDYID", "USUBJID", setdiff(colnames(ADTTE), colnames(ADSL)))],
              by = c("USUBJID", "STUDYID")
            )
          })
        })
      })
    }
  )
)

data <- teal_data_module(
  once = FALSE,
  ui = function(id) {
    ns <- NS(id)
    tagList(
      numericInput(ns("obs"), "Number of observations to show", 1000),
      actionButton(ns("submit"), label = "Submit")
    )
  },
  server = function(id, ...) {
    moduleServer(id, function(input, output, session) {
      logger::log_trace("example_module_transform2 initializing.")
      eventReactive(input$submit, {
        data <- teal_data() |>
          within(
            {
              logger::log_trace("Loading data")
              ADSL <- head(teal.data::rADSL, n = n)
              ADTTE <- teal.data::rADTTE
              aaa <- iris
              CO2 <- CO2
              factors <- names(Filter(isTRUE, vapply(CO2, is.factor, logical(1L))))
              CO2[factors] <- lapply(CO2[factors], as.character)
            },
            n = as.numeric(input$obs)
          )
        join_keys(data) <- default_cdisc_join_keys[c("ADSL", "ADTTE")]
        teal.data::datanames(data) <- c("CO2", "ADTTE", "aaa", "ADSL")
        data
      })
    })
  }
)

app <- teal::init(
  data = data,
  modules = list(
    example_module("insufficient", datanames = c("ADSL", "insufficient")),
    example_module("anl from transform", dataname = "ANL", transformers = anl_transformer),
    example_module("anl (fails)", datanames = "ANL")
  ),
  filter = teal_slices(
    teal_slice("ADSL", "SEX"),
    teal_slice("ADSL", "AGE", selected = c(18L, 65L)),
    include_varnames = list(
      ADSL = c("SEX", "AGE")
    )
  )
)

runApp(app)

@gogonzo gogonzo added the core label Aug 8, 2024
R/teal_data_utils.R Outdated Show resolved Hide resolved
R/teal_data_utils.R Outdated Show resolved Hide resolved
R/teal_data_utils.R Outdated Show resolved Hide resolved
R/zzz.R Outdated Show resolved Hide resolved
R/teal_data_utils.R Outdated Show resolved Hide resolved
R/teal_data_utils.R Outdated Show resolved Hide resolved
R/zzz.R Outdated Show resolved Hide resolved
@m7pr
Copy link
Contributor

m7pr commented Aug 9, 2024

Hey @gogonzo I left 2 comments. Would you be able to resolve conflicts?

R/teal_data_utils.R Outdated Show resolved Hide resolved
@m7pr
Copy link
Contributor

m7pr commented Aug 9, 2024

Hey, I can confirm I see the error on feature branch, but not on this branch

Main Feature Branch

on_feature_branch

This Branch

on_this_branch

Devtools::test()

There are still 3 checks failining but I know you are aware

── Failed tests ────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
Failure (test-module_teal.R:537:9): receives all objects from @env except _raw when module$datanames = "all" and @datanames is empty
teal.data::datanames((modules_output$module_1())()) not identical to c("iris", "mtcars", "swiss").
Lengths differ: 8 is not 3
Backtrace:1. ├─shiny::testServer(...) at test-module_teal.R:522:5
  2. │ ├─shiny:::withMockContext(...)
  3. │ │ ├─shiny::isolate(...)
  4. │ │ │ ├─shiny::..stacktraceoff..(...)
  5. │ │ │ └─ctx$run(...)
  6. │ │ │   ├─promises::with_promise_domain(...)
  7. │ │ │   │ └─domain$wrapSync(expr)
  8. │ │ │   ├─shiny::withReactiveDomain(...)
  9. │ │ │   │ └─promises::with_promise_domain(...)
 10. │ │ │   │   └─domain$wrapSync(expr)
 11. │ │ │   │     └─base::force(expr)
 12. │ │ │   └─env$runWith(self, func)
 13. │ │ │     └─shiny (local) contextFunc()
 14. │ │ │       └─shiny::..stacktraceon..(expr)
 15. │ │ ├─shiny::withReactiveDomain(...)
 16. │ │ │ └─promises::with_promise_domain(...)
 17. │ │ │   └─domain$wrapSync(expr)
 18. │ │ │     └─base::force(expr)
 19. │ │ └─withr::with_options(...)
 20. │ │   └─base::force(code)
 21. │ └─rlang::eval_tidy(quosure, mask, rlang::caller_env())
 22. └─testthat::expect_identical(...) at test-module_teal.R:537:9

Failure (test-module_teal.R:691:9): receives extra transform datasets if module$datanames == 'all' and @datanames empty
teal.data::datanames((modules_output$module_1())()) not identical to c("iris", "mtcars", "swiss").
Lengths differ: 5 is not 3
Backtrace:1. ├─shiny::testServer(...) at test-module_teal.R:660:5
  2. │ ├─shiny:::withMockContext(...)
  3. │ │ ├─shiny::isolate(...)
  4. │ │ │ ├─shiny::..stacktraceoff..(...)
  5. │ │ │ └─ctx$run(...)
  6. │ │ │   ├─promises::with_promise_domain(...)
  7. │ │ │   │ └─domain$wrapSync(expr)
  8. │ │ │   ├─shiny::withReactiveDomain(...)
  9. │ │ │   │ └─promises::with_promise_domain(...)
 10. │ │ │   │   └─domain$wrapSync(expr)
 11. │ │ │   │     └─base::force(expr)
 12. │ │ │   └─env$runWith(self, func)
 13. │ │ │     └─shiny (local) contextFunc()
 14. │ │ │       └─shiny::..stacktraceon..(expr)
 15. │ │ ├─shiny::withReactiveDomain(...)
 16. │ │ │ └─promises::with_promise_domain(...)
 17. │ │ │   └─domain$wrapSync(expr)
 18. │ │ │     └─base::force(expr)
 19. │ │ └─withr::with_options(...)
 20. │ │   └─base::force(code)
 21. │ └─rlang::eval_tidy(quosure, mask, rlang::caller_env())
 22. └─testthat::expect_identical(...) at test-module_teal.R:691:9

Failure (test-module_teal.R:1709:9): displays Obs only column if all datasets have no join keys
module_output_table(output, "module_1") not identical to data.frame(...).
Attributes: < Componentrow.names: Numeric: lengths (4, 2) differ >
ComponentData Name: Lengths (4, 2) differ (string compare on first 2)
ComponentData Name: 1 string mismatch
ComponentObs: Lengths (4, 2) differ (string compare on first 2)
ComponentObs: 1 string mismatch
Backtrace:1. ├─shiny::testServer(...) at test-module_teal.R:1696:5
  2. │ ├─shiny:::withMockContext(...)
  3. │ │ ├─shiny::isolate(...)
  4. │ │ │ ├─shiny::..stacktraceoff..(...)
  5. │ │ │ └─ctx$run(...)
  6. │ │ │   ├─promises::with_promise_domain(...)
  7. │ │ │   │ └─domain$wrapSync(expr)
  8. │ │ │   ├─shiny::withReactiveDomain(...)
  9. │ │ │   │ └─promises::with_promise_domain(...)
 10. │ │ │   │   └─domain$wrapSync(expr)
 11. │ │ │   │     └─base::force(expr)
 12. │ │ │   └─env$runWith(self, func)
 13. │ │ │     └─shiny (local) contextFunc()
 14. │ │ │       └─shiny::..stacktraceon..(expr)
 15. │ │ ├─shiny::withReactiveDomain(...)
 16. │ │ │ └─promises::with_promise_domain(...)
 17. │ │ │   └─domain$wrapSync(expr)
 18. │ │ │     └─base::force(expr)
 19. │ │ └─withr::with_options(...)
 20. │ │   └─base::force(code)
 21. │ └─rlang::eval_tidy(quosure, mask, rlang::caller_env())
 22. └─testthat::expect_identical(...) at test-module_teal.R:1709:9

[ FAIL 3 | WARN 0 | SKIP 49 | PASS 450 ]

@gogonzo
Copy link
Contributor Author

gogonzo commented Aug 9, 2024

There are still 3 checks failining [but I know you are aware]

fixed

@m7pr
Copy link
Contributor

m7pr commented Aug 9, 2024

I started the Check Github Action manually for this branch https://github.com/insightsengineering/teal/actions/runs/10319726556

Copy link
Contributor

@m7pr m7pr left a comment

Choose a reason for hiding this comment

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

Ok, the check is green - there was only one styling issue that I fixed
https://github.com/insightsengineering/teal/actions/runs/10319726556
Ready to be merged

@gogonzo gogonzo merged commit 202dbbd into 669_insertUI@main Aug 12, 2024
1 check passed
@gogonzo gogonzo deleted the all_datanames_to_transform@669_insertUI@main branch August 12, 2024 05:46
@github-actions github-actions bot locked and limited conversation to collaborators Aug 12, 2024
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.

3 participants