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

API simplification #2

Closed
13 tasks done
pawelru opened this issue Nov 16, 2021 · 3 comments
Closed
13 tasks done

API simplification #2

pawelru opened this issue Nov 16, 2021 · 3 comments
Labels

Comments

@pawelru
Copy link
Collaborator

pawelru commented Nov 16, 2021

Here is the list of planned actions (don't confuse with suggestions). This list might grow once we agree on other "unresolved" yet parts of the teal. The whole point of this milestone is to simplify API by:

  • making it easy to call the teal app.
  • making it easy to apply teal components to any shiny app.

1. Simplify teal components

Image

Final objective for first iteration of the API simplification is to call following module:

tm_module_srv <- function(data <list of reactives or list of data.frames>, filter_panel <filterPanelAPI>) {
  initial_chunks <- reactive({
    chunks_new(env = data, expr = get_expr(data))
  })
  ...
  extracts <- data_extract_srv(data, ...)
  merged_call <- data_merge_srv(extracts, get_join_keys(data), ...)
  
  another_chunk <- reactive({
    chunks_push(merged_call())
  })
  ...
  
  show_rcode(expr = final_chunk$get_expr())
}

To allow above model, we need to provide list of data.frames (or MAE) to data argument instead of passing datasets (FilteredData)

# somewhere in teal::init
data <- lapply(<module datanames>, FilteredData$get_reactive_data)

Another objective is to remove TealData frrom FilteredData dependencies.

  • FIlteredData should not depend on the TealData/TealDataset.
  • teal_module(s) should not depend on the FIlteredData

During the refactor we need to make sure that we won't break everything. I suggest to still send FilteredData if the datasets argument is used in the module.

@nikolas-burkoff
Copy link

nikolas-burkoff commented Apr 6, 2022

Can we also add simplifications such as insightsengineering/teal.transform#2 (or a replacement of it) and also possibly the equivalent for tmc (moving the data model out of the modules and into the cdisc object) so that specifying applications becomes significantly easier

@kpagacz
Copy link

kpagacz commented May 19, 2022

+1 on simplifying des

@donyunardi
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants