-
-
Notifications
You must be signed in to change notification settings - Fork 41
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
Changes from 27 commits
c9887c9
e7a3526
e9df325
c2e84eb
a8f85ca
dc3d7e3
08c7214
221976a
55a5a80
51b3f79
e43c76a
f0c1cc6
df885f3
a324e21
3ac84f9
e11acd7
0bb2e70
9244d33
d6b8d2f
0ce0eb5
71c1501
0c52e1a
9b7de0f
640c27b
7a451d3
82fb25a
cfb27bc
c3adb74
147c02f
cd292ea
e80c8ce
724dcdb
a98682d
507fbc3
0f7accd
0836745
e511517
bbaff1f
2a38182
971c7d5
72abf29
457ae2c
9d63d40
f84ad0b
7d5ccc3
6570474
8a0675f
422efa6
8e0cc87
2c928b2
54f05ba
0b12f7b
f00e181
019f26c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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" | ||
) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. to add: (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()] | ||
|
@@ -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( | ||
|
@@ -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() | ||
|
@@ -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")) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are you sure? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
|
@@ -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( | ||
|
There was a problem hiding this comment.
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 theteal_data
class name.There was a problem hiding this comment.
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 withteal_module_landing
andteal_module_previewer
. Unless this is not an extension ofteam_module
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 asmodule
argument value ofinit()
- it is meant to be adata
argument value instead. IMHO we should keep them separate to avoid confusion.There was a problem hiding this comment.
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:
tm_
and they output"teal_module"
classAnd we have additional two special modules which are "inheriting" from
"teal_module"
:landing_popup_module()
reporter_previewer_module()
I support
teal_data_module
, do we all agree here?There was a problem hiding this comment.
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
overteal_module_data
There was a problem hiding this comment.
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.