Skip to content

Commit

Permalink
forbid "all" in teal_transform_module (#1381)
Browse files Browse the repository at this point in the history
Closes #1347 

Additionally, changed `transforms` to `transformers` in the
`ui/srv_transform_data()` documentation as there was a param-name
mismatch with `module()`
  • Loading branch information
gogonzo authored Oct 17, 2024
1 parent 9c183f8 commit a3829c4
Show file tree
Hide file tree
Showing 11 changed files with 119 additions and 181 deletions.
6 changes: 2 additions & 4 deletions R/module_nested_tabs.R
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,7 @@ ui_teal_module.teal_module <- function(id, modules, depth = 0L) {
width = 3,
ui_data_summary(ns("data_summary")),
ui_filter_data(ns("filter_panel")),
if (length(modules$transformers) > 0 && !isTRUE(attr(modules$transformers, "custom_ui"))) {
ui_transform_data(ns("data_transform"), transforms = modules$transformers, class = "well")
},
ui_transform_data(ns("data_transform"), transformers = modules$transformers, class = "well"),
class = "teal_secondary_col"
)
)
Expand Down Expand Up @@ -272,7 +270,7 @@ srv_teal_module.teal_module <- function(id,
transformed_teal_data <- srv_transform_data(
"data_transform",
data = filtered_teal_data,
transforms = modules$transformers,
transformers = modules$transformers,
modules = modules,
is_transformer_failed = is_transformer_failed
)
Expand Down
30 changes: 14 additions & 16 deletions R/module_transform_data.R
Original file line number Diff line number Diff line change
Expand Up @@ -13,18 +13,19 @@
NULL

#' @rdname module_transform_data
ui_transform_data <- function(id, transforms, class = "well") {
ui_transform_data <- function(id, transformers = list(), class = "well") {
checkmate::assert_string(id)
checkmate::assert_list(transforms, "teal_transform_module", null.ok = TRUE)
checkmate::assert_list(transformers, "teal_transform_module")

ns <- NS(id)
labels <- lapply(transforms, function(x) attr(x, "label"))
labels <- lapply(transformers, function(x) attr(x, "label"))
ids <- get_unique_labels(labels)
names(transforms) <- ids
names(transformers) <- ids

lapply(
names(transforms),
names(transformers),
function(name) {
data_mod <- transforms[[name]]
data_mod <- transformers[[name]]
wrapper_id <- ns(sprintf("wrapper_%s", name))
div( # todo: accordion?
# class .teal_validated changes the color of the boarder on error in ui_validate_reactive_teal_data
Expand All @@ -44,37 +45,34 @@ ui_transform_data <- function(id, transforms, class = "well") {
),
div(
id = wrapper_id,
ui_teal_data(id = ns(name), data_module = transforms[[name]]$ui)
ui_teal_data(id = ns(name), data_module = transformers[[name]]$ui)
)
)
}
)
}

#' @rdname module_transform_data
srv_transform_data <- function(id, data, transforms, modules, is_transformer_failed = reactiveValues()) {
srv_transform_data <- function(id, data, transformers = list(), modules, is_transformer_failed = reactiveValues()) {
checkmate::assert_string(id)
assert_reactive(data)
checkmate::assert_list(transforms, "teal_transform_module", null.ok = TRUE)
checkmate::assert_list(transformers, "teal_transform_module")
checkmate::assert_class(modules, "teal_module")
if (length(transforms) == 0L) {
return(data)
}
labels <- lapply(transforms, function(x) attr(x, "label"))
labels <- lapply(transformers, function(x) attr(x, "label"))
ids <- get_unique_labels(labels)
names(transforms) <- ids
names(transformers) <- ids
moduleServer(id, function(input, output, session) {
logger::log_debug("srv_teal_data_modules initializing.")
Reduce(
function(previous_result, name) {
srv_teal_data(
id = name,
data_module = function(id) transforms[[name]]$server(id, previous_result),
data_module = function(id) transformers[[name]]$server(id, previous_result),
modules = modules,
is_transformer_failed = is_transformer_failed
)
},
x = names(transforms),
x = names(transformers),
init = data
)
})
Expand Down
57 changes: 32 additions & 25 deletions R/modules.R
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,29 @@ setOldClass("teal_modules")
#' because they are used by the `mapping` argument of [teal_slices()]
#' and the report previewer module [reporter_previewer_module()], respectively.
#'
#' # Restricting datasets used by `teal_module`:
#' The `datanames` argument controls which datasets are used by the module’s server. These datasets,
#' passed via server's `data` argument, are the only ones shown in the module's tab.
#'
#' When `datanames` is set to `"all"`, all datasets in the data object are treated as relevant.
#' However, this may include unnecessary datasets, such as:
#' - Proxy variables for column modifications
#' - Temporary datasets used to create final versions
#' - Connection objects
#'
#' To exclude irrelevant datasets, use the [set_datanames()] function to change `datanames` from
#' `"all"` to specific names. Trying to modify non-`"all"` values with [set_datanames()] will result
#' in a warning. Datasets with names starting with . are ignored globally unless explicitly listed
#' in `datanames`.
#'
#' # `datanames` with `transformers`
#' When transformers are specified, their `datanames` are added to the module’s `datanames`, which
#' changes the behavior as follows:
#' - If `module(datanames)` is `NULL` and the `transformers` have defined `datanames`, the sidebar
#' will appear showing the `transformers`' datasets, instead of being hidden.
#' - If `module(datanames)` is set to specific values and any `transformer` has `datanames = "all"`,
#' the module may receive extra datasets that could be unnecessary
#'
#' @param label (`character(1)`) Label shown in the navigation item for the module or module group.
#' For `modules()` defaults to `"root"`. See `Details`.
#' @param server (`function`) `shiny` module with following arguments:
Expand All @@ -42,40 +65,24 @@ setOldClass("teal_modules")
#' - `...` (optional) When provided, `ui_args` elements will be passed to the module named argument
#' or to the `...`.
#' @param filters (`character`) Deprecated. Use `datanames` instead.
#' @param datanames (`character`) Names of the datasets that are relevant for the item.
#' The keyword `"all"` provides all datasets available in `data` passed to `teal` application.
#' `NULL` will hide the filter panel.
#' @param datanames (`character`) Names of the datasets relevant to the item.
#' There are 2 reserved values that have specific behaviors:
#' - The keyword `"all"` includes all datasets available in the data passed to the teal application.
#' - `NULL` hides the sidebar panel completely.
#' - If `transformers` are specified, their `datanames` are automatically added to this `datanames`
#' argument.
#' @param server_args (named `list`) with additional arguments passed on to the server function.
#' @param ui_args (named `list`) with additional arguments passed on to the UI function.
#' @param x (`teal_module` or `teal_modules`) Object to format/print.
#' @param indent (`integer(1)`) Indention level; each nested element is indented one level more.
#' @param transformers (`list` of `teal_data_module`) that will be applied to transform the data.
#' Each transform module UI will appear in the `teal` application, unless the `custom_ui` attribute is set on the list.
#' If so, the module developer is responsible to display the UI in the module itself. `datanames` of the `transformers`
#' will be added to the `datanames`.
#' Each transform module UI will appear in the `teal`'s sidebar panel.
#' Transformers' `datanames` are added to the `datanames`. See [teal_transform_module()].
#'
#' When the transformation does not have sufficient input data, the resulting data will fallback
#' to the last successful transform or, in case there are none, to the filtered data.
#' @param ...
#' - For `modules()`: (`teal_module` or `teal_modules`) Objects to wrap into a tab.
#' - For `format()` and `print()`: Arguments passed to other methods.
#'
#' @section `datanames`:
#' The module's `datanames` argument determines a subset of datasets from the `data` object, as specified in the
#' server function argument, to be presented in the module. Datasets displayed in the filter panel will be limited
#' to this subset.
#' When `datanames` is set to `"all"`, all available datasets in the `data` object are considered relevant for the
#' module. However, setting `datanames` argument to `"all"` might include datasets that are irrelevant for the module,
#' for example:
#' - Proxy variables used for modifying columns.
#' - Modified copies of datasets used to create a final dataset.
#' - Connection objects.
#' To prevent these irrelevant datasets from appearing in the module, use the [set_datanames()] function on the
#' [module] or [modules()] to change the `datanames` from `"all"` to specific dataset names. Attempting to change
#' `datanames` values that was not set to `"all"` using [set_datanames()] will be ignored with a warning.
#'
#' Additionally, datasets with names starting with `.` are ignored when `datanames` is set to `"all"`.
#'
#' @return
#' `module()` returns an object of class `teal_module`.
#'
Expand Down Expand Up @@ -264,7 +271,7 @@ module <- function(label = "module",
}
checkmate::assert_list(transformers, types = "teal_transform_module")
transformer_datanames <- unlist(lapply(transformers, attr, "datanames"))
combined_datanames <- if (identical(datanames, "all") || any(sapply(transformer_datanames, identical, "all"))) {
combined_datanames <- if (identical(datanames, "all")) {
"all"
} else {
union(datanames, transformer_datanames)
Expand Down
14 changes: 10 additions & 4 deletions R/teal_data_module.R
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,8 @@ teal_data_module <- function(ui, server, label = "data module", once = TRUE) {
#' unexpected behavior.
#' See `vignettes("data-transform-as-shiny-module")` for more information.
#' @param datanames (`character`)
#' Names of the datasets that are relevant for the module. The
#' filter panel will only display filters for specified `datanames`. The keyword `"all"` will show
#' filters of all datasets. `datanames` will be automatically appended to the [modules()] `datanames`.
#' Names of the datasets that are relevant for this module to evaluate. If set to `character(0)`
#' then module would receive [modules()] `datanames`.
#' @examples
#' my_transformers <- list(
#' teal_transform_module(
Expand Down Expand Up @@ -144,10 +143,17 @@ teal_data_module <- function(ui, server, label = "data module", once = TRUE) {
teal_transform_module <- function(ui = function(id) NULL,
server = function(id, data) data,
label = "transform module",
datanames = "all") {
datanames = character(0)) {
checkmate::assert_function(ui, args = "id", nargs = 1)
checkmate::assert_function(server, args = c("id", "data"), nargs = 2)
checkmate::assert_string(label)
checkmate::assert_character(datanames)
if (identical(datanames, "all")) {
stop(
"teal_transform_module can't have datanames property equal to 'all'. Set `datanames = character(0)` instead.",
call. = FALSE
)
}
structure(
list(
ui = ui,
Expand Down
19 changes: 10 additions & 9 deletions man/example_module.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 6 additions & 2 deletions man/module_transform_data.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

58 changes: 34 additions & 24 deletions man/teal_modules.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 3 additions & 4 deletions man/teal_transform_module.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit a3829c4

Please sign in to comment.