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 decorators for tm_missing_data #809

930 changes: 560 additions & 370 deletions R/tm_missing_data.R

Large diffs are not rendered by default.

109 changes: 109 additions & 0 deletions R/utils.R
Original file line number Diff line number Diff line change
Expand Up @@ -280,3 +280,112 @@ assert_single_selection <- function(x,
}
invisible(TRUE)
}

#' Wrappers around `srv_transform_teal_data` that allows to decorate the data
#' @inheritParams teal::srv_transform_teal_data
#' @param expr (`expression` or `reactive`) to evaluate on the output of the decoration.
#' When an expression it must be inline code. See [within()]
#' Default is `NULL` which won't evaluate any appending code.
#' @details
#' `srv_decorate_teal_data` is a wrapper around `srv_transform_teal_data` that
#' allows to decorate the data with additional expressions.
#' When original `teal_data` object is in error state, it will show that error
#' first.
#'
#' @keywords internal
srv_decorate_teal_data <- function(id, data, decorators, expr) {
assert_reactive(data)
checkmate::assert_list(decorators, "teal_transform_module")

missing_expr <- missing(expr)
if (!missing_expr) {
expr <- rlang::enexpr(expr)
}

moduleServer(id, function(input, output, session) {
decorated_output <- srv_transform_teal_data("inner", data = data, transformators = decorators)

reactive({
# ensure original errors are displayed and `eval_code` is never executed with NULL
req(data(), decorated_output())
if (missing_expr) {
decorated_output()
} else {
eval_code(decorated_output(), expr)
}
})
})
}

#' @rdname srv_decorate_teal_data
#' @details
#' `ui_decorate_teal_data` is a wrapper around `ui_transform_teal_data`.
#' @keywords internal
ui_decorate_teal_data <- function(id, decorators, ...) {
teal::ui_transform_teal_data(NS(id, "inner"), transformators = decorators, ...)
}

#' Internal function to check if decorators is a valid object
#' @noRd
check_decorators <- function(x, names = NULL, null.ok = FALSE) {
checkmate::qassert(null.ok, "B1")

check_message <- checkmate::check_list(
x,
null.ok = null.ok,
names = "named"
)

if (!is.null(names)) {
check_message <- if (isTRUE(check_message)) {
out_message <- checkmate::check_names(names(x), subset.of = c("default", names))
# see https://github.com/insightsengineering/teal.logger/issues/101
if (isTRUE(out_message)) {
out_message
} else {
gsub("\\{", "(", gsub("\\}", ")", out_message))
}
} else {
check_message
}
}

if (!isTRUE(check_message)) {
return(check_message)
}

valid_elements <- vapply(
x,
checkmate::test_list,
types = "teal_transform_module",
null.ok = TRUE,
FUN.VALUE = logical(1L)
)

if (all(valid_elements)) {
return(TRUE)
}

"May only contain the type 'teal_transform_module' or a named list of 'teal_transform_module'."
}

#' Internal assertion on decorators
#' @noRd
assert_decorators <- checkmate::makeAssertionFunction(check_decorators)

#' Subset decorators based on the scope
#'
#' `default` is a protected decorator name that is always included in the output,
#' if it exists
#'
#' @param scope (`character`) a character vector of decorator names to include.
#' @param decorators (named `list`) of list decorators to subset.
#'
#' @return A flat list with all decorators to include.
#' It can be an empty list if none of the scope exists in `decorators` argument.
#' @keywords internal
subset_decorators <- function(scope, decorators) {
Copy link
Contributor

Choose a reason for hiding this comment

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

After A LOT of debugging I solved the issue on this module and I was rethinking our decision on relying on the same decorators for all "outputs".

I'm proposing we allow 3 types of values in decorators argument (similar to the initial code from a week and half ago):

  1. list of teal_transform_module (keep current list-like approach)
  2. Named list that can allow for customizations
    • list(default = list(...)) is protected and applies to all
    • list(summary_plot = list(...)) only applies decorator for summary_plot
  3. Also allow named list of teal_transform_module

We could limit to just 1. and 2., or even just 2.. WDYT?

Why?

It seems odd to have all UIs on tm_missing_data, in particular, having plot-like decorators UI that do nothing on a table output.

Note: the PR also extracts the qenv generation from the main shiny module into smaller and logic-only modules. I opted for using modules instead of just passing input to keep with good Shiny practices.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@averissimo yeah, so I think we could meet and rethink. I actually am having still various thoughs on how we should to it, depending on how the module is built. I think the named list of decorators would be the most appriopiate. I wonder how that changes the server logic.

Thanks for working on this module and having this fixed

checkmate::assert_character(scope)
scope <- intersect(union("default", scope), names(decorators))
c(list(), unlist(decorators[scope], recursive = FALSE))
}
32 changes: 32 additions & 0 deletions man/srv_decorate_teal_data.Rd

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

22 changes: 22 additions & 0 deletions man/subset_decorators.Rd

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

8 changes: 4 additions & 4 deletions man/tm_a_pca.Rd

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

8 changes: 4 additions & 4 deletions man/tm_a_regression.Rd

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

Loading
Loading