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

DDL refactor #168

Closed
gogonzo opened this issue Sep 26, 2023 · 0 comments · Fixed by insightsengineering/teal#957
Closed

DDL refactor #168

gogonzo opened this issue Sep 26, 2023 · 0 comments · Fixed by insightsengineering/teal#957
Assignees
Labels

Comments

@gogonzo
Copy link
Contributor

gogonzo commented Sep 26, 2023

🚧 WIP description... 🚧

We need to decide about final design of DDL (delayed data loading). There are several opinions of which arguments should be exposed, how they should be specified - ie what app developer should be responsible of.
Regardless of final solution, whole design can be divided on several elements:

  1. ui where app users will provide their inputs (username, password, submit button, selecting database etc.)
  2. server to manage input values and process of creating final tdata object
  3. code - code to pull data with optional arguments
  4. Execution of dynamic code - code where optional arguments are substituted with values taken from (ui) inputs. For example authorize(login, password)
  5. mask - list of arguments being inserted into the code for sake of reproducibility
  6. Creating tdata object from data environment created in (4) and code created in (5)

tdata-DDL process


1. Specifying ui

What level of customization we should allow. On one side of the spectrum user could specify just ui without server without any ns:

argument consequences
ddl(
   textInput("login", "Login"),
   passwordInput("pass", "Password"),
   actionButton("submit"),
   ...
)
Proposed initially to have ui without server with simplest version of inputs as possible. It accepts simple list of inputs but custom ui needs even more internal code to handle inputs nested in div.
ddl(
   ui = function(id) {
     ns <- NS(id)
     div(
       textInput("login", "Login"),
       passwordInput("pass", "Password")
       actionButton("submit")
     )
   }
)
Fully configurable shiny module

2. Specifying server

argument consequences
server hardcoded Not possible to make dynamic inputs. Dynamic inputs possible only through js/conditionalPanel (on the UI side).
function(id) {
  moduleServer(id, function(input, output, session)) {
    eventReactive(input$submit, {
       ddl_run(...)
     })
  }
Possible to create dynamic inputs and control the process of code execution and generating tdata object. Possible to make some default so user can cover basic scenario without server.

3. Way of specifying code

argument consequences
function(input) {
authorize(login = input$login, password = input$password)
data <- read_data(...)
}
Here we refer directly to the shiny input
quote({
authorize(login = login, password = password)
data <- read_data(...)
})
Here we refet to the name of the input element. Names matched also with mask

4. Execution of the code

In both scenarios it is a part of a server - code execution is triggered by relevant event (submit button). Function which executes a code is called in a server, so when the server argument is exposed developer needs to call this function explicitly. App developer needs to return tdata object from the server. Be aware that server might have a default the same as hardcoded one in an alternative solution.

moduleServer { 
  eventReactive(input$submit, {
    ddl_run(...) # calling function to execute the code
  })
}

5. mask

We have collective agreement regarding mask (i.e offline_args) argument.

6. postprocess_fun

This part is related to the server (2). There are some cases that some authorization is done through module and not by the code (3), which creates a gap in the output code. To fix this we need to add missing code when we create a tdata object.


After reading above consider different proposition for a simple case:

ddl(
   code = function(input) {
      authorize(user = input$user, password = input$password)
      data <- read_data()
  },
  mask = list(
    user = quote(askpass::askpass()), 
    password = quote(askpass::askpass())
   ),
  datanames = c("adsl", "adtte"),
  textInput("user", "username"),
  passwordInput("pass", "password"),
  actionButton("submit", "authorize") 
)
ddl(
   code = quote({
      authorize(user = user, password = password)
      data <- read_data()
  },
  mask = list(
    user = quote(askpass::askpass()), 
    password = quote(askpass::askpass())
   ),
  datanames = c("adsl", "adtte"),
  ui = function(id) {
      ns <- NS(id)
      div(
         textInput(ns("user"), "username"),
         passwordInput(ns("pass"), "password"),
         actionButton(ns("submit"), "authorize")
      )
  },
) # server and postprocess_fun have defaults which can be similar to hardcoded ones in alternative solution.

Consider more complicated case where we want to customize ui and server. Below example which can't be done with alternative solution.

ddl(
  ...,
  ui = function(id) {
      ns <- NS(id)
       div(
         id = ns("authorization_div")
         textInput(ns("user"), "username"),
         passwordInput(ns("pass"), "password"),
         actionButton(ns("authorize"), "authorize")
      ),
      div(
        id = ns("data_div"),
        selectInput(ns("study"), "Select study"),
        actionButton("load", "Load data")
      )
  },
  server = function(id, x) {
      moduleServer(id, function(input, output, session) {
          is_authorized <- eventReactive(input$authorize, {
             # authorize module
          })
          
          observeEvent(is_authorized(), {
            if (is_authorized()) shinyjs::show("data_div") else shinyjs::hide("data_div")
          })
          
          eventReactive(input$load, {
            ddl_run(x)
          })
      })
  },
)
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 a pull request may close this issue.

1 participant