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

1299 show insufficient datanames error only when transformers are missing #1309

Conversation

m7pr
Copy link
Contributor

@m7pr m7pr commented Aug 12, 2024

Fixes #1299 but blocked by #1298

averissimo and others added 12 commits August 9, 2024 16:16
…s_error@669_insertUI@main

* origin/669_insertUI@main:
  tests: update tests with modified transform code
  tests: add test for expected behavior on fallback and recovery
  fix: trigger .fallback when upstream data changes
  fix: correct the broken test logic
  chore: fix spellcheck
  [skip roxygen] [skip vbump] Roxygen Man Pages Auto Update
  [skip style] [skip vbump] Restyle files
  Exclude the "_raw" datasets only if the original dataset exists (#1305)
Changed the way how datanames are passed between teal_data instances
Changes:

1. Adds the section on how to place the transform UI in the custom
position inside the module.
2. Formats the `_pkgdown.yml` file (No change made to the file, just
styling)
3. Extend the `example_module()` to handle the selection when datasets
change (This can happen when using DDL to change datasets completely,
such an example will be added to `teal.gallery`)

---------

Signed-off-by: Vedha Viyash <[email protected]>
Co-authored-by: Marcin <[email protected]>
Co-authored-by: m7pr <[email protected]>
Co-authored-by: Dony Unardi <[email protected]>
@m7pr m7pr added the core label Aug 12, 2024
gogonzo and others added 2 commits August 12, 2024 12:48
Part of #1253

### Changes description

- `check_modules_datanames` returns a string and HTML generator for:
    - **string**: to be used with logger 
    - **HTML**: function to be used in teal UI
- Message is generated in the same way. This adds complexity, but is
consistent
- `c("one", "two", "three")` renders as "one, two and three" (note the
comma and `and`)
- In the module context it doesn't show the current module label

<details>

<summary>Sample app</summary>

```r
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")

my_transformers <- list(
  teal_transform_module(
    label = "reactive ADSL",
    ui = function(id) {
      ns <- NS(id)
      tagList(
        div("Some UI for transform (merge)"),
        actionButton(ns("btn"), "Reload data")
      )
    },
    server = function(id, data) {
      moduleServer(id, function(input, output, session) {
        eventReactive(input$btn, {
          data()
        })
      })
    }
  ),
  teal_transform_module(
    label = "Keep first 6 from IRIS",
    ui = function(id) {
      ns <- NS(id)
      div(
        span("Some UI for transform (1)"),
        textInput(ns("obs"), label = "Number of rows", value = 6)
      )
    },
    server = function(id, data) {
      moduleServer(id, function(input, output, session) {
        reactive({
          req(data())
          obs <- as.numeric(input$obs)
          if (!is.finite(obs)) stop("NOT NUMERIC.")
          within(data(), iris <- head(iris, n), n = as.numeric(input$obs))
        })
      })
    }
  ),
  teal_transform_module(
    label = "Keep first 6 from ADTTE",
    ui = function(id) div("Some UI for transform 2"),
    server = function(id, data) {
      moduleServer(id, function(input, output, session) {
        reactive({
          req(data())
          within(data(), ADTTE <- head(ADTTE))
        })
      })
    }
  )
)

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
              iris <- 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("ADSL", "ADTTE", "iris", "CO2")
        data
      })
    })
  }
)

teal::init(
  data = data,
  modules = list(
    example_module("mod-1", datanames = "all"),
    example_module("mod-2", transformers = my_transformers, datanames = c("ADSL", "ADTTE", "iris", "elo")),
    modules(
      label = "sub-modules",
      example_module("mod-2-sub1", transformers = my_transformers, datanames = c("ADSL", "ADTTE", "iris", "elo", "elo2")),
      example_module("mod-2-sub2", transformers = my_transformers, datanames = c("ADSL", "ADTTE", "iris", "elo"))
    ),
    example_module("mod-2", transformers = my_transformers[2:3])
  ),
  filter = teal_slices(
    teal_slice("ADSL", "SEX"),
    teal_slice("ADSL", "AGE", selected = c(18L, 65L))
  )
) |>
  runApp()
```

</details>


![image](https://github.com/user-attachments/assets/9a6c09a6-2ce4-4c2b-b7f6-0cce7ab8670c)


![image](https://github.com/user-attachments/assets/2b4a8dd1-f7e7-44f8-80d3-9cb45dd3909b)
Base automatically changed from datanames_error@669_insertUI@main to 669_insertUI@main August 12, 2024 10:59
Comment on lines +136 to +139
extra_datanames <- unique(c(
setdiff(modules$datanames, c("all", datanames)),
setdiff(unlist(lapply(modules$transformers, function(x) x$datanames)), c("all", datanames))
))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Once this is finished #1298
This check can be extended

@m7pr m7pr changed the title extend modules datanames check with their respective transformers datanames 1299 show insufficient datanames error only when transformers are missing Aug 12, 2024
@m7pr m7pr added the blocked label Aug 12, 2024
Base automatically changed from 669_insertUI@main to main August 13, 2024 03:49
@m7pr
Copy link
Contributor Author

m7pr commented Aug 14, 2024

Closing in favour of #1319

@m7pr m7pr closed this Aug 14, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Aug 14, 2024
@insights-engineering-bot insights-engineering-bot deleted the 1299_check_transformers_datanames@datanames_error@669_insertUI@main branch November 17, 2024 03:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Error/warning message when data is not sufficient and no teal transform modules
4 participants