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

Introduce the new DDL #957

Merged
merged 54 commits into from
Nov 13, 2023
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
c9887c9
approach 3
gogonzo Nov 1, 2023
e7a3526
[skip actions] Roxygen Man Pages Auto Update
dependabot-preview[bot] Nov 1, 2023
e9df325
ddl_login_password wrapper
gogonzo Nov 1, 2023
c2e84eb
[skip actions] Roxygen Man Pages Auto Update
dependabot-preview[bot] Nov 1, 2023
a8f85ca
remove redundant
gogonzo Nov 1, 2023
dc3d7e3
[skip actions] Roxygen Man Pages Auto Update
dependabot-preview[bot] Nov 1, 2023
08c7214
fixes
gogonzo Nov 2, 2023
221976a
:O
gogonzo Nov 2, 2023
55a5a80
modifying class to be defacto simpler version of `teal_module`
gogonzo Nov 3, 2023
51b3f79
[skip actions] Roxygen Man Pages Auto Update
dependabot-preview[bot] Nov 3, 2023
e43c76a
need to quote to avoid eval of language objects
gogonzo Nov 3, 2023
f0c1cc6
allow no server_args
gogonzo Nov 3, 2023
df885f3
change assertion to list(ui, server)!
gogonzo Nov 3, 2023
a324e21
tighten up asserts
gogonzo Nov 3, 2023
3ac84f9
fix asserts in teal with splash
gogonzo Nov 3, 2023
e11acd7
example ddl
gogonzo Nov 3, 2023
0bb2e70
[skip actions] Roxygen Man Pages Auto Update
dependabot-preview[bot] Nov 3, 2023
9244d33
review suggestions
gogonzo Nov 6, 2023
d6b8d2f
[skip actions] Roxygen Man Pages Auto Update
dependabot-preview[bot] Nov 6, 2023
0ce0eb5
fix notifications
gogonzo Nov 6, 2023
71c1501
[skip actions] Roxygen Man Pages Auto Update
dependabot-preview[bot] Nov 6, 2023
0c52e1a
removing delayed_data and ddl
gogonzo Nov 6, 2023
9b7de0f
add test for teal::init
gogonzo Nov 6, 2023
640c27b
- introduce `data_module`
gogonzo Nov 8, 2023
7a451d3
Merge 640c27bb8816d38081f1d2a5b6cd0adf1f406d6b into 60755551478f0f666…
gogonzo Nov 8, 2023
82fb25a
[skip actions] Restyle files
github-actions[bot] Nov 8, 2023
cfb27bc
add vignette
gogonzo Nov 8, 2023
c3adb74
Merge cfb27bcd90783cae615234f26b39879afc812579 into 60755551478f0f666…
gogonzo Nov 8, 2023
147c02f
[skip actions] Restyle files
github-actions[bot] Nov 8, 2023
cd292ea
empty
gogonzo Nov 8, 2023
e80c8ce
data_module -> teal_data_module
gogonzo Nov 8, 2023
724dcdb
WIP tests
gogonzo Nov 8, 2023
a98682d
- add asserts on datanames in teal::init
gogonzo Nov 9, 2023
507fbc3
lint
gogonzo Nov 9, 2023
0f7accd
edit vignette
Nov 9, 2023
0836745
Merge 0f7accd87e42a3bd6c4e0c041f8448806ac67029 into 60755551478f0f666…
gogonzo Nov 9, 2023
e511517
[skip actions] Restyle files
github-actions[bot] Nov 9, 2023
bbaff1f
update documentation for teal_data_module
Nov 9, 2023
2a38182
WIP review
gogonzo Nov 9, 2023
971c7d5
protect teal from teal_data_module errors
gogonzo Nov 10, 2023
72abf29
@kartikayakirar
gogonzo Nov 10, 2023
457ae2c
fix typo
chlebowa Nov 10, 2023
9d63d40
add NEWS entry and reorganize in proper sections
gogonzo Nov 10, 2023
f84ad0b
@chlebowa @ruckip review
gogonzo Nov 10, 2023
7d5ccc3
review
gogonzo Nov 10, 2023
6570474
fix error handling when simple error in teal_data_module
gogonzo Nov 10, 2023
8a0675f
change error message when teal_data_module doesn't return reactive
gogonzo Nov 10, 2023
422efa6
review
gogonzo Nov 10, 2023
8e0cc87
@ruckip review
gogonzo Nov 13, 2023
2c928b2
@ruckip review
gogonzo Nov 13, 2023
54f05ba
update snapshot manager documentation
Nov 13, 2023
0b12f7b
link to the vignette
gogonzo Nov 13, 2023
f00e181
adding link to vignette and fix error message
gogonzo Nov 13, 2023
019f26c
lintr
gogonzo Nov 13, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ LazyData: true
Roxygen: list(markdown = TRUE)
RoxygenNote: 7.2.3
Collate:
'data_module.R'
'dummy_functions.R'
'get_rcode_utils.R'
'include_css_js.R'
Expand Down
1 change: 1 addition & 0 deletions NAMESPACE
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ S3method(ui_nested_tabs,teal_modules)
export("%>%")
export(TealReportCard)
export(as.teal_slices)
export(data_module)
export(example_module)
export(get_code_tdata)
export(get_metadata)
Expand Down
40 changes: 40 additions & 0 deletions R/data_module.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
#' Data module
#'
#' Data input for `teal::init` in form of a module
#'
#' @param ui (`function(id)`)\cr
#' `shiny` `ui` module with `id` argument
#' @param server (`function(id)`)\cr
#' `shiny` server function with `id` as argument. Module should return reactive `teal_data`.
#' @examples
#' data <- data_module(
#' ui = function(id) {
#' ns <- NS(id)
#' actionButton(ns("submit"), label = "Load data")
#' },
#' server = function(id) {
#' moduleServer(id, function(input, output, session) {
#' eventReactive(input$submit, {
#' data <- within(
#' teal.data::teal_data(),
#' {
#' dataset1 <- iris
#' dataset2 <- mtcars
#' }
#' )
#' teal.data::datanames(data) <- c("iris", "mtcars")
#'
#' data
#' })
#' })
#' }
#' )
#' @export
data_module <- function(ui, server) {
checkmate::assert_function(ui, args = "id", nargs = 1)
checkmate::assert_function(server, args = "id", nargs = 1)
structure(
list(ui = ui, server = server),
class = "data_module"
Copy link
Contributor

Choose a reason for hiding this comment

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

teal_data_module? That would fit nicely to the teal_data class name.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would opt for teal_module_data to be consistent with teal_module_landing and teal_module_previewer. Unless this is not an extension of team_module.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would opt for teal_module_data to be consistent with teal_module_landing and teal_module_previewer. Unless this is not an extension of team_module.

Disagree. This class is different to the one you mentioned. First of all it does not inherit from teal_module. Second of all this is not meant to be passed as module argument value of init() - it is meant to be a data argument value instead. IMHO we should keep them separate to avoid confusion.

Copy link
Contributor Author

@gogonzo gogonzo Nov 8, 2023

Choose a reason for hiding this comment

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

We have some unwritten convention:

  • Analytical module function have a prefix tm_ and they output "teal_module" class

And we have additional two special modules which are "inheriting" from "teal_module":

  • "teal_module_landing" produced by landing_popup_module()
  • "teal_module_previewer" produced by reporter_previewer_module()

I support teal_data_module, do we all agree here?

Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer teal_data_module over teal_module_data

Copy link
Contributor

Choose a reason for hiding this comment

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

Like I said, unless. I am pretty sure they will be confused anyway. Nomenclature throughout NEST is not entirely consistent and this difference will likely be seen as an inconsistency rather than a sign of a different entity.

I don't mind teal_data_module as such.

)
}
15 changes: 13 additions & 2 deletions R/dummy_functions.R
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ example_datasets <- function() { # nolint
#'
#' @description `r lifecycle::badge("experimental")`
#' @inheritParams module
#' @param src (`logical(1)`) whether to display reproducible R code in the module.
#' @return A `teal` module which can be included in the `modules` argument to [teal::init()].
#' @examples
#' app <- init(
Expand All @@ -88,21 +89,31 @@ example_datasets <- function() { # nolint
#' shinyApp(app$ui, app$server)
#' }
#' @export
example_module <- function(label = "example teal module", datanames = "all") {
chlebowa marked this conversation as resolved.
Show resolved Hide resolved
example_module <- function(label = "example teal module", datanames = "all", src = TRUE) {
checkmate::assert_string(label)
module(
label,
server = function(id, data) {
checkmate::assert_class(data, "tdata")
moduleServer(id, function(input, output, session) {
output$text <- renderPrint(data[[input$dataname]]())
teal.widgets::verbatim_popup_srv(
id = "rcode",
verbatim_content = attr(data, "code")(),
title = "Association Plot"
)
})
},
ui = function(id, data) {
ns <- NS(id)
teal.widgets::standard_layout(
output = verbatimTextOutput(ns("text")),
encoding = selectInput(ns("dataname"), "Choose a dataset", choices = names(data))
encoding = div(
selectInput(ns("dataname"), "Choose a dataset", choices = names(data)),
if (src) {
teal.widgets::verbatim_popup_ui(ns("rcode"), "Show R code")
}
)
)
},
datanames = datanames
Expand Down
44 changes: 9 additions & 35 deletions R/init.R
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@
#' an end-user, don't use this function, but instead this module.
#'
#' @param data (`TealData` or `TealDataset` or `TealDatasetConnector` or `list` or `data.frame`
#' or `MultiAssayExperiment`, `teal_data`)\cr
#' or `MultiAssayExperiment`, `teal_data`, `data_module`)\cr
#' `R6` object as returned by [teal.data::cdisc_data()], [teal.data::teal_data()],
#' [teal.data::cdisc_dataset()], [teal.data::dataset()], [teal.data::dataset_connector()] or
#' [teal.data::cdisc_dataset_connector()] or a single `data.frame` or a `MultiAssayExperiment`
#' [teal.data::cdisc_dataset_connector()] or [teal::data_module()] or a single `data.frame` or a `MultiAssayExperiment`
#' or a list of the previous objects or function returning a named list.
Copy link
Contributor

Choose a reason for hiding this comment

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

to add:
or teal_data_module object created by teal_data_module()

(the latter with link)

#' NOTE: teal does not guarantee reproducibility of the code when names of the list elements
#' do not match the original object names. To ensure reproducibility please use [teal.data::teal_data()]
Expand Down Expand Up @@ -114,11 +114,11 @@ init <- function(data,
footer = tags$p(),
id = character(0)) {
logger::log_trace("init initializing teal app with: data ({ class(data)[1] }).")

if (!inherits(data, c("TealData", "teal_data"))) {
if (!inherits(data, c("TealData", "teal_data", "data_module"))) {
data <- teal.data::to_relational_data(data = data)
}
checkmate::assert_multi_class(data, c("TealData", "teal_data"))

checkmate::assert_multi_class(data, c("TealData", "teal_data", "data_module"))
checkmate::assert_multi_class(modules, c("teal_module", "list", "teal_modules"))
checkmate::assert_string(title, null.ok = TRUE)
checkmate::assert(
Expand All @@ -142,26 +142,12 @@ init <- function(data,
if (length(landing) > 1L) stop("Only one `landing_popup_module` can be used.")
modules <- drop_module(modules, "teal_module_landing")

# resolve modules datanames
datanames <- teal.data::get_dataname(data)
join_keys <- teal.data::get_join_keys(data)
modules <- resolve_modules_datanames(modules = modules, datanames = datanames, join_keys = join_keys)

if (!inherits(filter, "teal_slices")) {
checkmate::assert_subset(names(filter), choices = datanames)
# list_to_teal_slices is lifted from teal.slice package, see zzz.R
# This is a temporary measure and will be removed two release cycles from now (now meaning 0.13.0).
filter <- list_to_teal_slices(filter)
}
# convert teal.slice::teal_slices to teal::teal_slices
filter <- as.teal_slices(as.list(filter))

# Calculate app hash to ensure snapshot compatibility. See ?snapshot. Raw data must be extracted from environments.
hashables <- mget(c("data", "modules"))
hashables$data <- if (inherits(hashables$data, "teal_data")) {
as.list(hashables$data@env)
} else if (inherits(hashables$data, "ddl")) {
attr(hashables$data, "code")
} else if (inherits(data, "data_module")) {
# what?
chlebowa marked this conversation as resolved.
Show resolved Hide resolved
} else if (hashables$data$is_pulled()) {
sapply(get_dataname(hashables$data), simplify = FALSE, function(dn) {
hashables$data$get_dataset(dn)$get_raw_data()
Expand All @@ -172,20 +158,8 @@ init <- function(data,

attr(filter, "app_id") <- rlang::hash(hashables)

# check teal_slices
for (i in seq_along(filter)) {
dataname_i <- shiny::isolate(filter[[i]]$dataname)
if (!dataname_i %in% datanames) {
stop(
sprintf(
"filter[[%s]] has a different dataname than available in a 'data':\n %s not in %s",
i,
dataname_i,
toString(datanames)
)
)
}
}
# convert teal.slice::teal_slices to teal::teal_slices
filter <- as.teal_slices(as.list(filter))

if (isTRUE(attr(filter, "module_specific"))) {
module_names <- unlist(c(module_labels(modules), "global_filters"))
Expand Down
9 changes: 6 additions & 3 deletions R/module_nested_tabs.R
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,7 @@ ui_nested_tabs.teal_module <- function(id, modules, datasets, depth = 0L, is_mod
checkmate::assert_class(datasets, class = "FilteredData")
ns <- NS(id)

args <- isolate(teal.transform::resolve_delayed(modules$ui_args, datasets))
Copy link
Contributor

Choose a reason for hiding this comment

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

are you sure?
if yes - is this the only place where we use this function?
if yes - shall we remove it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, too early for this. I was testing something and pushed by mistake. Reverting.

FYI we will remove resolve_delay from ui functions anyway soon.

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool. Yes - I am expecting this to be removed but was surprised this happened so fast :P

args <- c(list(id = ns("module")), args)
args <- c(list(id = ns("module")), modules$ui_args)

if (is_arg_used(modules$ui, "datasets")) {
args <- c(args, datasets = datasets)
Expand Down Expand Up @@ -297,7 +296,11 @@ srv_nested_tabs.teal_module <- function(id, datasets, modules, is_module_specifi
checkmate::assert_class(datasets, "FilteredData")
checkmate::assert_class(trigger_data, "reactiveVal")

datanames <- if (is.null(module$datanames)) datasets$datanames() else module$datanames
datanames <- if (is.null(module$datanames) || identical(module$datanames, "all")) {
datasets$datanames()
} else {
unique(module$datanames) # todo: include parents! unique shouldn't be needed here!
}

# list of reactive filtered data
data <- sapply(
Expand Down
8 changes: 7 additions & 1 deletion R/module_tabs_with_filters.R
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,13 @@ srv_tabs_with_filters <- function(id,
)

if (!is_module_specific) {
active_datanames <- reactive(active_module()$datanames)
active_datanames <- reactive({
if (identical(active_module()$datanames, "all")) {
singleton$datanames()
} else {
active_module()$datanames
}
})
singleton <- unlist(datasets)[[1]]
singleton$srv_filter_panel("filter_panel", active_datanames = active_datanames)

Expand Down
39 changes: 22 additions & 17 deletions R/module_teal.R
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,11 @@ srv_teal <- function(id, modules, raw_data, filter = teal_slices()) {
}
)

reporter <- teal.reporter::Reporter$new()
if (is_arg_used(modules, "reporter") && length(extract_module(modules, "teal_module_previewer")) == 0) {
modules <- append_module(modules, reporter_previewer_module())
}

env <- environment()
datasets_reactive <- eventReactive(raw_data(), {
env$progress <- shiny::Progress$new(session)
Expand All @@ -171,6 +176,7 @@ srv_teal <- function(id, modules, raw_data, filter = teal_slices()) {
# Singleton starts with only global filters active.
filter_global <- Filter(function(x) x$id %in% attr(filter, "mapping")$global_filters, filter)
datasets_singleton$set_filter_state(filter_global)

module_datasets <- function(modules) {
if (inherits(modules, "teal_modules")) {
datasets <- lapply(modules$children, module_datasets)
Expand All @@ -180,11 +186,16 @@ srv_teal <- function(id, modules, raw_data, filter = teal_slices()) {
} else if (isTRUE(attr(filter, "module_specific"))) {
# we should create FilteredData even if modules$datanames is null
# null controls a display of filter panel but data should be still passed
datanames <- if (is.null(modules$datanames)) teal.data::get_dataname(raw_data()) else modules$datanames
# todo: subset tdata object to datanames
datanames <- if (is.null(modules$datanames) || modules$datanames == "all") {
include_parent_datanames(raw_data()@datanames, raw_data()@join_keys) # todo: use methods instead
gogonzo marked this conversation as resolved.
Show resolved Hide resolved
} else {
modules$datanames
}
# todo: subset teal_data to datanames
datasets_module <- teal_data_to_filtered_data(raw_data())

# set initial filters
# - filtering filters for this module
slices <- Filter(x = filter, f = function(x) {
x$id %in% unique(unlist(attr(filter, "mapping")[c(modules$label, "global_filters")])) &&
x$dataname %in% datanames
Expand All @@ -199,29 +210,23 @@ srv_teal <- function(id, modules, raw_data, filter = teal_slices()) {
datasets_singleton
}
}
datasets <- module_datasets(modules)

logger::log_trace("srv_teal@4 Raw Data transferred to FilteredData.")
datasets
module_datasets(modules)
})

reporter <- teal.reporter::Reporter$new()
if (is_arg_used(modules, "reporter") && length(extract_module(modules, "teal_module_previewer")) == 0) {
modules <- append_module(modules, reporter_previewer_module())
}

# Replace splash / welcome screen once data is loaded ----
# ignoreNULL to not trigger at the beginning when data is NULL
# just handle it once because data obtained through delayed loading should
# usually not change afterwards
# if restored from bookmarked state, `filter` is ignored
observeEvent(datasets_reactive(), ignoreNULL = TRUE, once = TRUE, {
gogonzo marked this conversation as resolved.
Show resolved Hide resolved

observeEvent(datasets_reactive(), {
logger::log_trace("srv_teal@5 setting main ui after data was pulled")
env$progress$set(0.5, message = "Setting up main UI")
on.exit(env$progress$close())
# main_ui_container contains splash screen first and we remove it and replace it by the real UI
env$progress$set(0.5, message = "Setting up main UI")
datasets <- datasets_reactive()

removeUI(sprintf("#%s:first-child", session$ns("main_ui_container")))
# main_ui_container contains splash screen first and we remove it and replace it by the real UI
removeUI(sprintf("#%s > div:nth-child(1)", session$ns("main_ui_container")))
insertUI(
selector = paste0("#", session$ns("main_ui_container")),
where = "beforeEnd",
Expand All @@ -230,7 +235,7 @@ srv_teal <- function(id, modules, raw_data, filter = teal_slices()) {
ui = div(ui_tabs_with_filters(
session$ns("main_ui"),
modules = modules,
datasets = datasets_reactive(),
datasets = datasets,
filter = filter
)),
# needed so that the UI inputs are available and can be immediately updated, otherwise, updating may not
Expand All @@ -242,7 +247,7 @@ srv_teal <- function(id, modules, raw_data, filter = teal_slices()) {
# registered once (calling server functions twice would trigger observers twice each time)
active_module <- srv_tabs_with_filters(
id = "main_ui",
datasets = datasets_reactive(),
datasets = datasets,
modules = modules,
reporter = reporter,
filter = filter
Expand Down
Loading
Loading