Introduce the new DDL

[gogonzo]

Part of #937
Closes #895

Another variation for ddl. Key changes:

• teal::init additionally accepts list(ui = , server = ). Both only with id argument.
• delayed_data() function creates list(ui=, server=) and allows to call server function with additional arguments. delayed_data accepts additional arguments in ... and they are used in server() evaluation. These ... must be named and matching server formals.
• We expose eval_and_mask(code, input, input_mask) function to support evaluation-with-masking. We need to think about naming of a function and its arguments.
• Since eval_and_mask is used in the similar way as eval_code and within I decided to not use input$xyz for masking but xyz instead. Please share your feelings about this.

App code

library(scda)
pkgload::load_all("teal")

# initial data
data <- teal_data() |> within({
  iris <- iris
  mtcars <- mtcars
})

code <- quote({
  cat("\nusername: ", username, "\npassword: ", password) # mock connection function
  ADSL <- scda::synthetic_cdisc_data("latest")$adsl
  ADTTE <- scda::synthetic_cdisc_data("latest")$adtte
})

env_mask <- list(
  username = quote(askpass::askpass()),
  password = quote(askpass::askpass())
)

ddl_data <- list(
  ui = function(id) {
    ns <- NS(id)
    tagList(
      textInput(ns("username"), label = "Username"),
      passwordInput(ns("password"), label = "Password"),
      actionButton(ns("submit"), label = "Submit")
    )
  },
  server = function(id) {
    moduleServer(id, function(input, output, session) {
      eventReactive(input$submit, {
        masked_data <- data |> eval_and_mask(code = code, env = reactiveValuesToList(input), env_mask = env_mask)
        # iris, mtcars set in preprocessing; ADSL and ADTTE during teal session
        teal.data::datanames(masked_data) <- c("iris", "mtcars", "ADSL", "ADTTE")
        masked_data
      })
    })
  }
)

app <- teal::init(
  data = ddl_data,
  modules = list(
    teal.modules.general::tm_data_table("Data Table")
  )
)

runApp(app)

[m7pr]

I don't prefer this approach over #955 because here we introduce a function that has a confusing name with teal.transform package. Maybe teal_data_tranfsorm so that it is clear it is meant for teal_data transformations, which would be an extension of teal_data object. I don't like the way ui and server are split and created separately. When they were created together in one module, like in #955, it was clearer to follow what is their purpouse.

But as @pawelru mentioned in here #955 (comment)

(transform module) should be an extension

we can have #955 as basic specification of teal_data() module and a teal_data_transform module for any extra customized adjustments.

[vedhav]

Is it possible to have an implementation where we do not pass a teal_data object with it's own code and additional masked code for DDL? I think it would be great if the DDL code is passed as part of the teal_data object.

I would also consider adding masking as part of the teal_data creation step. This way, the masking is not part of the UI/Server logic. And one can theoretically mask without creating UI and just pass the masking secrets from env variable and just pass it as data = teal_data() with having the power to pass delayed data without UI.

code passed along with `teal_data`

ui <- function(id) {
  ns <- NS(id)
  tagList(
    textInput(ns("username"), label = "Username"),
    passwordInput(ns("password"), label = "Password"),
    actionButton(ns("submit"), label = "Submit")
  )
}

server <- function(id, data) {
  env_mask <- list(
    username = quote(askpass::askpass()),
    password = quote(askpass::askpass())
  )

  moduleServer(id, function(input, output, session) {
    eventReactive(input$submit, {
      masked_data <- data |> eval_and_mask(env = reactiveValuesToList(input), env_mask = env_mask)
    })
  })
}

app <- init(
  data = delayed_data(
    ui = ui,
    server = server,
    data = teal_data(
      iris = iris,
      code = quote({
        iris <- iris
        open_conn(username = username, password = password)
        ADSL <- scda::synthetic_cdisc_data("latest")$adsl
        ADTTE <- scda::synthetic_cdisc_data("latest")$adtte
      })
    )
  ),
  modules = modules(
    teal.modules.general::tm_data_table(label = "yolo")
  ),
  filter = teal_slices(
    teal_slice(dataname = "ADSL", varname = "AGE", selected = c(18, 60)),
    teal_slice(dataname = "inexisting", varname = "AGE")
  )
)

runApp(app)

[gogonzo]

Is it possible to have an implementation where we do not pass a teal_data object with it's own code and additional masked code for DDL?

Now, yes, just specify ui and server (only with id), and don't pass any data.

code

options(teal.log_level = "TRACE", teal.show_js_log = TRUE)
library(scda)
pkgload::load_all("teal")

ui <- function(id) {
  ns <- NS(id)
  tagList(
    textInput(ns("username"), label = "Username"),
    passwordInput(ns("password"), label = "Password"),
    actionButton(ns("submit"), label = "Submit")
  )
}

server <- function(id) {
  code <- quote({
    cat("\nusername: ", username, "\npassword: ", password) # mock connection function
    ADSL <- scda::synthetic_cdisc_data("latest")$adsl
    ADTTE <- scda::synthetic_cdisc_data("latest")$adtte
  })

  env_mask <- list(
    username = quote(askpass::askpass()),
    password = quote(askpass::askpass())
  )

  moduleServer(id, function(input, output, session) {
    eventReactive(input$submit, {
      masked_data <- teal_data() |> eval_and_mask(code = code, env = reactiveValuesToList(input), env_mask = env_mask)
    })
  })
}

app <- init(
  data = list(ui = ui, server = server),
  modules = modules(
    teal.modules.general::tm_data_table(label = "yolo")
  ),
  filter = teal_slices(
    teal_slice(dataname = "ADSL", varname = "AGE", selected = c(18, 60)),
    teal_slice(dataname = "inexisting", varname = "AGE")
  )
)

runApp(app)

[gogonzo]

@pawelru

Here is basic example with app using data = list(ui = ui, server = server)

app code

options(teal.log_level = "TRACE", teal.show_js_log = TRUE)
library(scda)
pkgload::load_all("teal")

ui <- function(id) {
  ns <- NS(id)
  tagList(
    textInput(ns("username"), label = "Username"),
    passwordInput(ns("password"), label = "Password"),
    actionButton(ns("submit"), label = "Submit")
  )
}

server <- function(id) {
  code <- quote({
    cat("\nusername: ", username, "\npassword: ", password) # mock connection function
    ADSL <- scda::synthetic_cdisc_data("latest")$adsl
    ADTTE <- scda::synthetic_cdisc_data("latest")$adtte
  })

  env_mask <- list(
    username = quote(askpass::askpass()),
    password = quote(askpass::askpass())
  )

  moduleServer(id, function(input, output, session) {
    eventReactive(input$submit, {
      masked_data <- teal_data() |> eval_and_mask(code = code, env = reactiveValuesToList(input), env_mask = env_mask)
    })
  })
}

app <- init(
  data = list(ui = ui, server = server),
  modules = modules(
    teal.modules.general::tm_data_table(label = "yolo")
  ),
  filter = teal_slices(
    teal_slice(dataname = "ADSL", varname = "AGE", selected = c(18, 60)),
    teal_slice(dataname = "inexisting", varname = "AGE")
  )
)

runApp(app)

[gogonzo]

I think there is an agreement that teal::init(data =) should accept also list(ui = , server = ). Everything build on the list is something extra, but assertion is (and should be) still valid.

I see three ways out:

1. We stay with list(ui, server) and we export only eval_and_mask(). ddl case will be exposed only in the vignette as a special case.
2. We can export delayed_data (name to decide) and expose "delayed data loading" in the documentation as an extension of delayed_data
3. We can export ddl <- function(code, input_mask, ui_server) (we need delayed_data to create such function).

[gogonzo]

Could be used in this way

app code

options(teal.log_level = "TRACE", teal.show_js_log = TRUE)
library(scda)
pkgload::load_all("teal")

ui <- function(id) {
  ns <- NS(id)
  tagList(
    textInput(ns("username"), label = "Username"),
    passwordInput(ns("password"), label = "Password"),
    actionButton(ns("submit"), label = "Submit")
  )
}

server <- function(id, code, input_mask) {
  moduleServer(id, function(input, output, session) {
    eventReactive(input$submit, {
      masked_data <- teal_data() |> eval_and_mask(code = code, input = input, input_mask = input_mask)
    })
  })
}

data <- ddl(
  code = quote({
    open_conn(username = username, password = password)
    ADSL <- scda::synthetic_cdisc_data("latest")$adsl
    ADTTE <- scda::synthetic_cdisc_data("latest")$adtte
  }),
  input_mask = list(
    username = quote(askpass::askpass()),
    password = quote(askpass::askpass())
  ),
  ui = ui,
  server = server
)

app <- init(
  data = data,
  modules = modules(
    teal.modules.general::tm_data_table(label = "yolo")
  ),
  filter = teal_slices(
    teal_slice(dataname = "ADSL", varname = "AGE", selected = c(18, 60)),
    teal_slice(dataname = "inexisting", varname = "AGE")
  )
)

runApp(app)

[gogonzo]

Exactly, I think the same

[kartikeyakirar]

We can export delayed_data (name to decide) and expose "delayed data loading" in the documentation as an extension of delayed_data

my preference would be exporting delayed_data as it might be more helpful to understand the class and concept itself to user.

[kartikeyakirar]

Not sure why I can not see my ques above so pasting here again,

Question: Do we require the function ddl()? It only utilize delayed_data with an additional argument . Perhaps it would be more suitable as additonal example in documentation or in the vignette?

[gogonzo]

my preference would be exporting delayed_data as it might be more helpful to understand the class and concept itself to user.

I have the same opinion. Besides, delayed_data might have much wider application

[m7pr]

I wasn't the fan of teal_transform which is now delayed_data. But I guess if this solves the purpose and has the most benefits I think we should expose. From the other hand DDL sounds like a very specific use-case, so I would be fine with this as well

We stay with list(ui, server) and we export only eval_and_mask(). ddl case will be exposed only in the vignette as a special case.

[m7pr]

I don't think exposing makes it easier to understand as there is server_formals, extra_args, server_args, extra_formals. If app developer understand that much of R and Shiny, would he be able to craft his own tool for his needs, instead of using teal? Even though I vote to expose

[gogonzo]

there is server_formals, extra_args, server_args, extra_formals

extra_args and extra_formals are just helpful to print relevant error message. There are server formals and ... which need to be matched. We need to throw meaningful message to alert that they don't match ;]

[m7pr]

I think dark smog clouds has overcome my brain today :)

[pawelru]

to add:
or teal_data_module object created by teal_data_module()

(the latter with link)

[pawelru]

just for safety -> add: else if inherits modules "teal_module" ... else stop(...)

[gogonzo]

added assert on modules to be teal_modules - it is enough as modules can't contain anything else.

[pawelru]

OK. I am done for this round. Please ping me after you finish pushing corrections.

[pawelru]

This is good. Thank you. Let's take hashing discussion elsewhere not to block everything that depends on this.