From f9bd19475e4dfca51a0e1e44513dbdbd5448f29d Mon Sep 17 00:00:00 2001 From: Pawel Rucki <12943682+pawelru@users.noreply.github.com> Date: Mon, 18 Mar 2024 09:53:57 +0100 Subject: [PATCH] options for strict tests; few enhancements (#225) - part of https://github.com/insightsengineering/coredev-tasks/issues/478 - please read this for more info about the implementation: https://github.com/insightsengineering/coredev-tasks/issues/478#issuecomment-1909912778 - fix detected errors - rm one big nolint block - rm `teal.widgets::` from vignette as this is redundant from within that repo Signed-off-by: Pawel Rucki <12943682+pawelru@users.noreply.github.com> --- R/draggable_buckets.R | 1 + R/ggplot2_args.R | 10 ++++---- R/nested_closeable_modal.R | 28 +++++++++++------------ R/plot_with_settings.R | 11 +++++---- R/verbatim_popup.R | 1 + man/draggable_buckets.Rd | 1 + man/ggplot2_args.Rd | 2 +- man/nested_closeable_modal.Rd | 26 ++++++++++----------- man/parse_ggplot2_args.Rd | 4 ++-- man/plot_with_settings.Rd | 11 +++++---- man/resolve_ggplot2_args.Rd | 4 ++-- man/verbatim_popup.Rd | 1 + tests/testthat/setup-options.R | 20 ++++++++++++++++ tests/testthat/test-table_with_settings.R | 2 +- vignettes/custom-ggplot2-arguments.Rmd | 24 +++++++++---------- 15 files changed, 86 insertions(+), 60 deletions(-) create mode 100644 tests/testthat/setup-options.R diff --git a/R/draggable_buckets.R b/R/draggable_buckets.R index 86c4be7d..9b996cc3 100644 --- a/R/draggable_buckets.R +++ b/R/draggable_buckets.R @@ -15,6 +15,7 @@ #' #' @examples #' library(shiny) +#' #' ui <- fluidPage( #' draggable_buckets("id", "Choices #1", c("a", "b"), c("bucket1", "bucket2")), #' draggable_buckets("id2", "Choices #2", letters, c("vowels", "consonants")), diff --git a/R/ggplot2_args.R b/R/ggplot2_args.R index dfb2e837..90d291cb 100644 --- a/R/ggplot2_args.R +++ b/R/ggplot2_args.R @@ -20,7 +20,7 @@ #' @export #' @examples #' ggplot2_args( -#' lab = list(title = "TITLE"), +#' labs = list(title = "TITLE"), #' theme = list(title = ggplot2::element_text(size = 20)) #' ) ggplot2_args <- function(labs = list(), theme = list()) { @@ -75,11 +75,11 @@ ggplot2_args <- function(labs = list(), theme = list()) { #' @examples #' resolve_ggplot2_args( #' user_plot = ggplot2_args( -#' lab = list(title = "TITLE"), +#' labs = list(title = "TITLE"), #' theme = list(title = ggplot2::element_text(size = 20)) #' ), #' user_default = ggplot2_args( -#' lab = list(x = "XLAB") +#' labs = list(x = "XLAB") #' ) #' ) resolve_ggplot2_args <- function(user_plot = ggplot2_args(), @@ -121,7 +121,7 @@ resolve_ggplot2_args <- function(user_plot = ggplot2_args(), #' @examples #' parse_ggplot2_args( #' resolve_ggplot2_args(ggplot2_args( -#' lab = list(title = "TITLE"), +#' labs = list(title = "TITLE"), #' theme = list(title = ggplot2::element_text(size = 20)) #' )) #' ) @@ -129,7 +129,7 @@ resolve_ggplot2_args <- function(user_plot = ggplot2_args(), #' parse_ggplot2_args( #' resolve_ggplot2_args( #' ggplot2_args( -#' lab = list(title = "TITLE"), +#' labs = list(title = "TITLE"), #' theme = list(title = ggplot2::element_text(size = 20)) #' ) #' ), diff --git a/R/nested_closeable_modal.R b/R/nested_closeable_modal.R index 0c080189..edcb87e2 100644 --- a/R/nested_closeable_modal.R +++ b/R/nested_closeable_modal.R @@ -15,13 +15,12 @@ #' @export #' #' @examples -#' # nolint start #' library(shiny) #' library(shinyjs) #' #' ui <- fluidPage( #' useShinyjs(), -#' actionButton("show_1", "$('#modal_1').modal('show')"), +#' actionButton("show_1", "$(\"#modal_1\").modal(\"show\")"), #' nested_closeable_modal( #' "modal_1", #' modal_args = list( @@ -31,7 +30,7 @@ #' footer = NULL #' ), #' tags$div( -#' "This modal can be closed by running", tags$code("$('#modal_1').modal('hide')"), +#' "This modal can be closed by running", tags$code("$(\"#modal_1\").modal(\"hide\")"), #' "in the JS console!", #' tags$br(), #' "Note that the second modal is placed right within this modal", @@ -39,8 +38,8 @@ #' "Alternatively, calling the", tags$code("removeModal()"), #' "will remove all the active modal popups", #' tags$br(), tags$br(), -#' actionButton("show_2", "$('#modal_2').modal('show')"), -#' actionButton("hide_1", "$('#modal_1').modal('hide')"), +#' actionButton("show_2", "$(\"#modal_2\").modal(\"show\")"), +#' actionButton("hide_1", "$(\"#modal_1\").modal(\"hide\")"), #' nested_closeable_modal( #' id = "modal_2", #' modal_args = list( @@ -50,35 +49,36 @@ #' easyClose = TRUE #' ), #' tags$div( -#' "This modal can be closed by running", tags$code("$('#modal_1').modal('hide')"), +#' "This modal can be closed by running", tags$code("$(\"#modal_1\").modal(\"hide\")"), #' "in the JS console!", #' "Note that removing the parent will remove the child. #' But, reopening will remember the open state of child", -#' actionButton("hide_2", "$('#modal_2').modal('hide')"), -#' actionButton("hide_all", "$('#modal_1').modal('hide')") +#' actionButton("hide_2", "$(\"#modal_2\").modal(\"hide\")"), +#' actionButton("hide_all", "$(\"#modal_1\").modal(\"hide\")") #' ) #' ) #' ) #' ) #' ) +#' #' server <- function(input, output) { #' observeEvent(input$show_1, { -#' runjs("$('#modal_1').modal('show')") +#' runjs("$(\"#modal_1\").modal(\"show\")") #' }) #' observeEvent(input$show_2, { -#' runjs("$('#modal_2').modal('show')") +#' runjs("$(\"#modal_2\").modal(\"show\")") #' }) #' observeEvent(c(input$hide_1, input$hide_all), { -#' runjs("$('#modal_1').modal('hide')") +#' runjs("$(\"#modal_1\").modal(\"hide\")") #' }) #' observeEvent(input$hide_2, { -#' runjs("$('#modal_2').modal('hide')") +#' runjs("$(\"#modal_2\").modal(\"hide\")") #' }) #' } +#' #' if (interactive()) { #' shinyApp(ui, server) #' } -#' # nolint end nested_closeable_modal <- function(id, ..., modal_args = list(easyClose = TRUE)) { checkmate::assert_string(id) checkmate::assert_list(modal_args) @@ -90,7 +90,7 @@ nested_closeable_modal <- function(id, ..., modal_args = list(easyClose = TRUE)) children("div")$ children("div")$ children("div")$ - sibling(".modal-footer")$ + siblings(".modal-footer")$ find("button")$ removeAttrs(c("data-dismiss", "data-bs-dismiss"))$ addAttrs(onclick = paste0("$('#", id, "').modal('hide');"))$ diff --git a/R/plot_with_settings.R b/R/plot_with_settings.R index 73ae94d6..dfe11d68 100644 --- a/R/plot_with_settings.R +++ b/R/plot_with_settings.R @@ -122,7 +122,7 @@ plot_with_settings_ui <- function(id) { #' #' server <- function(input, output, session) { #' plot_r <- reactive({ -#' ggplot(faithful, aes(x = waiting, y = eruptions)) + +#' ggplot(faithful, aes(x = .data$waiting, y = .data$eruptions)) + #' geom_point() #' }) #' @@ -140,6 +140,7 @@ plot_with_settings_ui <- function(id) { #' #' # Example using a function as input to plot_r #' library(lattice) +#' #' ui <- fluidPage( #' radioButtons("download_option", "Select the Option", list("ggplot", "trellis", "grob", "base")), #' plot_with_settings_ui( @@ -152,14 +153,14 @@ plot_with_settings_ui <- function(id) { #' plot_r <- function() { #' numbers <- seq_len(input$nums) #' if (input$download_option == "ggplot") { -#' ggplot(data.frame(n = numbers), aes(n)) + +#' ggplot(data.frame(n = numbers), aes(.data$n)) + #' geom_bar() #' } else if (input$download_option == "trellis") { #' densityplot(numbers) #' } else if (input$download_option == "grob") { #' tr_plot <- densityplot(numbers) #' ggplotGrob( -#' ggplot(data.frame(n = numbers), aes(n)) + +#' ggplot(data.frame(n = numbers), aes(.data$n)) + #' geom_bar() #' ) #' } else if (input$download_option == "base") { @@ -194,7 +195,7 @@ plot_with_settings_ui <- function(id) { #' #' server <- function(input, output, session) { #' plot_r <- reactive({ -#' ggplot(faithful, aes(x = waiting, y = eruptions)) + +#' ggplot(faithful, aes(x = .data$waiting, y = .data$eruptions)) + #' geom_point() #' }) #' @@ -231,7 +232,7 @@ plot_with_settings_ui <- function(id) { #' #' server <- function(input, output, session) { #' plot_r <- plot_r <- reactive( -#' ggplot(faithful, aes(x = waiting, y = eruptions)) + +#' ggplot(faithful, aes(x = .data$waiting, y = .data$eruptions)) + #' geom_point() #' ) #' diff --git a/R/verbatim_popup.R b/R/verbatim_popup.R index 49fe06ac..c4cf7dc2 100644 --- a/R/verbatim_popup.R +++ b/R/verbatim_popup.R @@ -14,6 +14,7 @@ #' #' @examples #' library(shiny) +#' #' ui <- fluidPage(verbatim_popup_ui("my_id", button_label = "Open popup")) #' srv <- function(input, output) { #' verbatim_popup_srv( diff --git a/man/draggable_buckets.Rd b/man/draggable_buckets.Rd index debba645..0cb5b3d9 100644 --- a/man/draggable_buckets.Rd +++ b/man/draggable_buckets.Rd @@ -28,6 +28,7 @@ A custom widget with draggable elements that can be put into buckets. } \examples{ library(shiny) + ui <- fluidPage( draggable_buckets("id", "Choices #1", c("a", "b"), c("bucket1", "bucket2")), draggable_buckets("id2", "Choices #2", letters, c("vowels", "consonants")), diff --git a/man/ggplot2_args.Rd b/man/ggplot2_args.Rd index d1d091af..23205f5b 100644 --- a/man/ggplot2_args.Rd +++ b/man/ggplot2_args.Rd @@ -26,7 +26,7 @@ For more details see the vignette: \code{vignette("custom-ggplot2-arguments", pa } \examples{ ggplot2_args( - lab = list(title = "TITLE"), + labs = list(title = "TITLE"), theme = list(title = ggplot2::element_text(size = 20)) ) } diff --git a/man/nested_closeable_modal.Rd b/man/nested_closeable_modal.Rd index 642c1fd6..ee206abe 100644 --- a/man/nested_closeable_modal.Rd +++ b/man/nested_closeable_modal.Rd @@ -25,13 +25,12 @@ Alternative to \code{shiny::modalDialog}. Create a nested modal popup that can b using \code{jQuery} and modal \code{id}, without disturbing the parent modal. } \examples{ -# nolint start library(shiny) library(shinyjs) ui <- fluidPage( useShinyjs(), - actionButton("show_1", "$('#modal_1').modal('show')"), + actionButton("show_1", "$(\"#modal_1\").modal(\"show\")"), nested_closeable_modal( "modal_1", modal_args = list( @@ -41,7 +40,7 @@ ui <- fluidPage( footer = NULL ), tags$div( - "This modal can be closed by running", tags$code("$('#modal_1').modal('hide')"), + "This modal can be closed by running", tags$code("$(\"#modal_1\").modal(\"hide\")"), "in the JS console!", tags$br(), "Note that the second modal is placed right within this modal", @@ -49,8 +48,8 @@ ui <- fluidPage( "Alternatively, calling the", tags$code("removeModal()"), "will remove all the active modal popups", tags$br(), tags$br(), - actionButton("show_2", "$('#modal_2').modal('show')"), - actionButton("hide_1", "$('#modal_1').modal('hide')"), + actionButton("show_2", "$(\"#modal_2\").modal(\"show\")"), + actionButton("hide_1", "$(\"#modal_1\").modal(\"hide\")"), nested_closeable_modal( id = "modal_2", modal_args = list( @@ -60,33 +59,34 @@ ui <- fluidPage( easyClose = TRUE ), tags$div( - "This modal can be closed by running", tags$code("$('#modal_1').modal('hide')"), + "This modal can be closed by running", tags$code("$(\"#modal_1\").modal(\"hide\")"), "in the JS console!", "Note that removing the parent will remove the child. But, reopening will remember the open state of child", - actionButton("hide_2", "$('#modal_2').modal('hide')"), - actionButton("hide_all", "$('#modal_1').modal('hide')") + actionButton("hide_2", "$(\"#modal_2\").modal(\"hide\")"), + actionButton("hide_all", "$(\"#modal_1\").modal(\"hide\")") ) ) ) ) ) + server <- function(input, output) { observeEvent(input$show_1, { - runjs("$('#modal_1').modal('show')") + runjs("$(\"#modal_1\").modal(\"show\")") }) observeEvent(input$show_2, { - runjs("$('#modal_2').modal('show')") + runjs("$(\"#modal_2\").modal(\"show\")") }) observeEvent(c(input$hide_1, input$hide_all), { - runjs("$('#modal_1').modal('hide')") + runjs("$(\"#modal_1\").modal(\"hide\")") }) observeEvent(input$hide_2, { - runjs("$('#modal_2').modal('hide')") + runjs("$(\"#modal_2\").modal(\"hide\")") }) } + if (interactive()) { shinyApp(ui, server) } -# nolint end } diff --git a/man/parse_ggplot2_args.Rd b/man/parse_ggplot2_args.Rd index 33c98412..55bd1499 100644 --- a/man/parse_ggplot2_args.Rd +++ b/man/parse_ggplot2_args.Rd @@ -27,7 +27,7 @@ A function to parse expression from the \code{ggplot2_args} object. \examples{ parse_ggplot2_args( resolve_ggplot2_args(ggplot2_args( - lab = list(title = "TITLE"), + labs = list(title = "TITLE"), theme = list(title = ggplot2::element_text(size = 20)) )) ) @@ -35,7 +35,7 @@ parse_ggplot2_args( parse_ggplot2_args( resolve_ggplot2_args( ggplot2_args( - lab = list(title = "TITLE"), + labs = list(title = "TITLE"), theme = list(title = ggplot2::element_text(size = 20)) ) ), diff --git a/man/plot_with_settings.Rd b/man/plot_with_settings.Rd index 792984cf..b508a5d1 100644 --- a/man/plot_with_settings.Rd +++ b/man/plot_with_settings.Rd @@ -91,7 +91,7 @@ ui <- fluidPage( server <- function(input, output, session) { plot_r <- reactive({ - ggplot(faithful, aes(x = waiting, y = eruptions)) + + ggplot(faithful, aes(x = .data$waiting, y = .data$eruptions)) + geom_point() }) @@ -109,6 +109,7 @@ if (interactive()) { # Example using a function as input to plot_r library(lattice) + ui <- fluidPage( radioButtons("download_option", "Select the Option", list("ggplot", "trellis", "grob", "base")), plot_with_settings_ui( @@ -121,14 +122,14 @@ server <- function(input, output, session) { plot_r <- function() { numbers <- seq_len(input$nums) if (input$download_option == "ggplot") { - ggplot(data.frame(n = numbers), aes(n)) + + ggplot(data.frame(n = numbers), aes(.data$n)) + geom_bar() } else if (input$download_option == "trellis") { densityplot(numbers) } else if (input$download_option == "grob") { tr_plot <- densityplot(numbers) ggplotGrob( - ggplot(data.frame(n = numbers), aes(n)) + + ggplot(data.frame(n = numbers), aes(.data$n)) + geom_bar() ) } else if (input$download_option == "base") { @@ -163,7 +164,7 @@ ui <- fluidPage( server <- function(input, output, session) { plot_r <- reactive({ - ggplot(faithful, aes(x = waiting, y = eruptions)) + + ggplot(faithful, aes(x = .data$waiting, y = .data$eruptions)) + geom_point() }) @@ -200,7 +201,7 @@ ui <- fluidPage( server <- function(input, output, session) { plot_r <- plot_r <- reactive( - ggplot(faithful, aes(x = waiting, y = eruptions)) + + ggplot(faithful, aes(x = .data$waiting, y = .data$eruptions)) + geom_point() ) diff --git a/man/resolve_ggplot2_args.Rd b/man/resolve_ggplot2_args.Rd index 6806b8f8..1321023d 100644 --- a/man/resolve_ggplot2_args.Rd +++ b/man/resolve_ggplot2_args.Rd @@ -48,11 +48,11 @@ Per plot (\code{user_plot}) and then default (\code{user_default}) setup. \examples{ resolve_ggplot2_args( user_plot = ggplot2_args( - lab = list(title = "TITLE"), + labs = list(title = "TITLE"), theme = list(title = ggplot2::element_text(size = 20)) ), user_default = ggplot2_args( - lab = list(x = "XLAB") + labs = list(x = "XLAB") ) ) } diff --git a/man/verbatim_popup.Rd b/man/verbatim_popup.Rd index dbe7271d..277ca15a 100644 --- a/man/verbatim_popup.Rd +++ b/man/verbatim_popup.Rd @@ -46,6 +46,7 @@ modal window with verbatim-styled text. } \examples{ library(shiny) + ui <- fluidPage(verbatim_popup_ui("my_id", button_label = "Open popup")) srv <- function(input, output) { verbatim_popup_srv( diff --git a/tests/testthat/setup-options.R b/tests/testthat/setup-options.R new file mode 100644 index 00000000..78be1f9b --- /dev/null +++ b/tests/testthat/setup-options.R @@ -0,0 +1,20 @@ +# `opts_partial_match_old` is left for exclusions due to partial matching in dependent packages (i.e. not fixable here) +# it might happen that it is not used right now, but it is left for possible future use +# use with: `withr::with_options(opts_partial_match_old, { ... })` inside the test +opts_partial_match_old <- list( + warnPartialMatchDollar = getOption("warnPartialMatchDollar"), + warnPartialMatchArgs = getOption("warnPartialMatchArgs"), + warnPartialMatchAttr = getOption("warnPartialMatchAttr") +) +opts_partial_match_new <- list( + warnPartialMatchDollar = TRUE, + warnPartialMatchArgs = TRUE, + warnPartialMatchAttr = TRUE +) + +if (isFALSE(getFromNamespace("on_cran", "testthat")()) && requireNamespace("withr", quietly = TRUE)) { + withr::local_options( + opts_partial_match_new, + .local_envir = testthat::teardown_env() + ) +} diff --git a/tests/testthat/test-table_with_settings.R b/tests/testthat/test-table_with_settings.R index ddbc6d6c..ca43c318 100644 --- a/tests/testthat/test-table_with_settings.R +++ b/tests/testthat/test-table_with_settings.R @@ -1,7 +1,7 @@ table_r <- shiny::reactive({ l <- rtables::basic_table() %>% rtables::split_cols_by("Species") %>% - rtables::analyze(c("Sepal.Length")) + rtables::analyze("Sepal.Length") rtables::build_table(l, iris) }) diff --git a/vignettes/custom-ggplot2-arguments.Rmd b/vignettes/custom-ggplot2-arguments.Rmd index 2fa795f5..afc55552 100644 --- a/vignettes/custom-ggplot2-arguments.Rmd +++ b/vignettes/custom-ggplot2-arguments.Rmd @@ -9,11 +9,11 @@ vignette: > %\VignetteEncoding{UTF-8} --- -This vignette will guide you through implementation of custom `ggplot2::labs` and `ggplot2::theme` for `ggplot2` graphics +This vignette will guide you through implementation of custom `ggplot2::labs` and `ggplot2::theme` for `ggplot2` graphics based modules. We will enable 2 ways of updating `ggplot2::labs` and `ggplot2::theme` by the end users. -The `ggplot2` specification could be updated with the `teal.ggplot2_args` `options` variable or a `ggplot2_args` -argument in a `tm_g_*` module. We still take into account default specification set up by the module creator in the +The `ggplot2` specification could be updated with the `teal.ggplot2_args` `options` variable or a `ggplot2_args` +argument in a `tm_g_*` module. We still take into account default specification set up by the module creator in the server function, which has the lowest priority. The implementation should consist of 5 steps: @@ -23,13 +23,13 @@ the `ggplot2_args(labs = list(), theme = list())` function for single plot. and `list(default = ggplot2_args(labs = list(), theme = list()))` multi-plot modules. 2. Adding a validation (e.g. `stopifnot` or `checkmate`) of the `ggplot2_args` argument to the `tm_*` function. The validation is more complex for multi-plot modules, where the `ggplot2_args` could be a `list`. -The module creator has to provide a list of plots names, which should be validated at this step and added to the `param` +The module creator has to provide a list of plots names, which should be validated at this step and added to the `param` field in `roxygen2`. For multi-plot modules the step `if (is_ggplot2_args) ggplot2_args <- list(default = ggplot2_args)` is recommended. 3. Aggregating and reducing all `ggplot2_args` sources with `resolve_ggplot2_args()`. 4. Usage of the `parse_ggplot2_args()` function which will parse inputs to list of expressions. -5. Adding the created expression to the last chunk of a plot. `Reduce(function(x, y) call("+", x, y), list(...)` +5. Adding the created expression to the last chunk of a plot. `Reduce(function(x, y) call("+", x, y), list(...)` function could be helpful at this step. The `resolve_ggplot2_args()` function picks the first non NULL value for each argument, checking in order: @@ -41,16 +41,16 @@ For multi-plot case, per plot (`user_plot`) and then default (`user_default`) se ### Additional topics -When a more complex `ggplot2` object has to be used inside the `teal.widgets::ggplot2_args` function, then a `base::quote` -function would prevent an object expansion in Show R Code. For example the `ggplot2::element_text` function returns a -complex object, then we should use code like -`teal.widgets::ggplot2_args(theme = list(plot.title = quote(ggplot2::element_text(size = 20))))` to prevent Show R Code +When a more complex `ggplot2` object has to be used inside the `ggplot2_args` function, then a `base::quote` +function would prevent an object expansion in Show R Code. For example the `ggplot2::element_text` function returns a +complex object, then we should use code like +`ggplot2_args(theme = list(plot.title = quote(ggplot2::element_text(size = 20))))` to prevent Show R Code expansion. -If you get a `promise already under evaluation: recursive default argument reference or earlier problems?` error, then +If you get a `promise already under evaluation: recursive default argument reference or earlier problems?` error, then probably your function argument has the same name as a function which is an input for it. -To solve the problem please use `::` to prefix it directly to a specific package, like -`new_fun <- function(ggplot2_args = teal.widgets::ggplot2_args())`. +To solve the problem please use `::` to prefix it directly to a specific package, like +`new_fun <- function(ggplot2_args = ggplot2_args())`. ## Loading libraries and data