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

Ensures required packages are in Imports section #774

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
35 changes: 17 additions & 18 deletions DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,29 @@ Depends:
teal.transform (>= 0.5.0)
Imports:
checkmate (>= 2.1.0),
colourpicker,
dplyr (>= 1.0.5),
DT (>= 0.13),
forcats (>= 1.0.0),
generics,
ggExtra,
ggpmisc (>= 0.4.3),
ggpp,
ggrepel,
goftest,
grid,
gridExtra,
htmlwidgets,
jsonlite,
lattice (>= 0.18-4),
MASS,
rtables (>= 0.6.8),
scales,
shinyjs,
shinyTree (>= 0.2.8),
shinyvalidate,
shinyWidgets (>= 0.5.1),
sparkline,
stats,
stringr (>= 1.4.1),
teal.code (>= 0.5.0),
Expand All @@ -53,28 +67,14 @@ Imports:
tools,
utils
Suggests:
broom (>= 0.7.10),
colourpicker,
ggExtra,
ggpmisc (>= 0.4.3),
ggpp,
ggrepel,
goftest,
gridExtra,
htmlwidgets,
jsonlite,
knitr (>= 1.42),
lattice (>= 0.18-4),
logger (>= 0.2.0),
MASS,
nestcolor (>= 0.1.0),
pkgload,
rlang (>= 1.0.0),
rmarkdown (>= 2.23),
rtables (>= 0.6.8),
rvest,
shinytest2,
sparkline,
testthat (>= 3.1.9),
withr (>= 2.0.0)
VignetteBuilder:
Expand All @@ -88,10 +88,9 @@ Config/Needs/verdepcheck: haleyjeppson/ggmosaic, tidyverse/ggplot2,
insightsengineering/teal.data, insightsengineering/teal.logger,
insightsengineering/teal.reporter, insightsengineering/teal.widgets,
insightsengineering/tern, tidyverse/tibble, tidyverse/tidyr,
tidymodels/broom, daattali/colourpicker, daattali/ggExtra,
aphalo/ggpmisc, aphalo/ggpp, slowkow/ggrepel, baddstats/goftest,
gridExtra, ramnathv/htmlwidgets, jeroen/jsonlite, yihui/knitr,
daroczig/logger, deepayan/lattice, MASS,
daattali/colourpicker, daattali/ggExtra, aphalo/ggpmisc, aphalo/ggpp,
slowkow/ggrepel, baddstats/goftest, gridExtra, ramnathv/htmlwidgets,
jeroen/jsonlite, yihui/knitr, daroczig/logger, deepayan/lattice, MASS,
insightsengineering/nestcolor, r-lib/rlang, rstudio/rmarkdown,
insightsengineering/rtables, tidyverse/rvest, sparkline,
rstudio/shinytest2, r-lib/testthat, r-lib/withr
Expand Down
14 changes: 2 additions & 12 deletions R/tm_g_distribution.R
Original file line number Diff line number Diff line change
Expand Up @@ -121,16 +121,6 @@ tm_g_distribution <- function(label = "Distribution Module",
post_output = NULL) {
message("Initializing tm_g_distribution")

# Requires Suggested packages
extra_packages <- c("ggpmisc", "ggpp", "goftest", "MASS", "broom")
missing_packages <- Filter(function(x) !requireNamespace(x, quietly = TRUE), extra_packages)
if (length(missing_packages) > 0L) {
stop(sprintf(
"Cannot load package(s): %s.\nInstall or restart your session.",
toString(missing_packages)
))
}

# Normalize the parameters
if (inherits(dist_var, "data_extract_spec")) dist_var <- list(dist_var)
if (inherits(strata_var, "data_extract_spec")) strata_var <- list(strata_var)
Expand Down Expand Up @@ -1176,7 +1166,7 @@ srv_distribution <- function(id,
expr = {
test_stats <- ANL %>%
dplyr::select(dist_var) %>%
with(., broom::glance(do.call(test, args))) %>%
with(., generics::glance(do.call(test, args))) %>%
dplyr::mutate_if(is.numeric, round, 3)
},
env = env
Expand All @@ -1190,7 +1180,7 @@ srv_distribution <- function(id,
test_stats <- ANL %>%
dplyr::select(dist_var, s_var, g_var) %>%
dplyr::group_by_at(dplyr::vars(dplyr::any_of(groups))) %>%
dplyr::do(tests = broom::glance(do.call(test, args))) %>%
dplyr::do(tests = generics::glance(do.call(test, args))) %>%
tidyr::unnest(tests) %>%
dplyr::mutate_if(is.numeric, round, 3)
},
Expand Down
10 changes: 0 additions & 10 deletions R/tm_g_scatterplot.R
Original file line number Diff line number Diff line change
Expand Up @@ -229,16 +229,6 @@ tm_g_scatterplot <- function(label = "Scatterplot",
ggplot2_args = teal.widgets::ggplot2_args()) {
message("Initializing tm_g_scatterplot")

# Requires Suggested packages
extra_packages <- c("ggpmisc", "ggExtra", "colourpicker")
missing_packages <- Filter(function(x) !requireNamespace(x, quietly = TRUE), extra_packages)
if (length(missing_packages) > 0L) {
stop(sprintf(
"Cannot load package(s): %s.\nInstall or restart your session.",
toString(missing_packages)
))
}

# Normalize the parameters
if (inherits(x, "data_extract_spec")) x <- list(x)
if (inherits(y, "data_extract_spec")) y <- list(y)
Expand Down
5 changes: 0 additions & 5 deletions R/tm_g_scatterplotmatrix.R
Original file line number Diff line number Diff line change
Expand Up @@ -164,11 +164,6 @@ tm_g_scatterplotmatrix <- function(label = "Scatterplot Matrix",
post_output = NULL) {
message("Initializing tm_g_scatterplotmatrix")

# Requires Suggested packages
if (!requireNamespace("lattice", quietly = TRUE)) {
stop("Cannot load lattice - please install the package or restart your session.")
}

# Normalize the parameters
if (inherits(variables, "data_extract_spec")) variables <- list(variables)

Expand Down
88 changes: 45 additions & 43 deletions R/tm_missing_data.R
Original file line number Diff line number Diff line change
Expand Up @@ -91,14 +91,6 @@ tm_missing_data <- function(label = "Missing data",
post_output = NULL) {
message("Initializing tm_missing_data")

# Requires Suggested packages
if (!requireNamespace("gridExtra", quietly = TRUE)) {
stop("Cannot load gridExtra - please install the package or restart your session.")
}
if (!requireNamespace("rlang", quietly = TRUE)) {
stop("Cannot load rlang - please install the package or restart your session.")
}

# Normalize the parameters
if (inherits(ggplot2_args, "ggplot2_args")) ggplot2_args <- list(default = ggplot2_args)

Expand Down Expand Up @@ -1135,6 +1127,13 @@ srv_missing_data <- function(id, data, reporter, filter_panel_api, dataname, par
ggtheme = input$ggtheme
)

# Unlikely that `rlang` is not available, new hashing may be expensive
hashing_function <- if (requireNamespace("rlang", quietly = TRUE)) {
quote(rlang::hash)
} else {
function(x) paste(as.integer(x), collapse = "")
Copy link
Contributor Author

@averissimo averissimo Aug 14, 2024

Choose a reason for hiding this comment

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

note: this hashing function gets a logical vector as input

}

teal.code::eval_code(
common_code_q(),
substitute(
Expand All @@ -1149,41 +1148,44 @@ srv_missing_data <- function(id, data, reporter, filter_panel_api, dataname, par
)
) %>%
teal.code::eval_code(
quote({
summary_plot_patients <- ANL[, c(parent_keys, analysis_vars)] %>%
dplyr::group_by_at(parent_keys) %>%
dplyr::mutate(id = dplyr::cur_group_id()) %>%
dplyr::ungroup() %>%
dplyr::group_by_at(c(parent_keys, "id")) %>%
dplyr::summarise_all(anyNA) %>%
dplyr::ungroup()

# order subjects by decreasing number of missing and then by
# missingness pattern (defined using sha1)
order_subjects <- summary_plot_patients %>%
dplyr::select(-"id", -dplyr::all_of(parent_keys)) %>%
dplyr::transmute(
id = dplyr::row_number(),
number_NA = apply(., 1, sum),
sha = apply(., 1, rlang::hash)
) %>%
dplyr::arrange(dplyr::desc(number_NA), sha) %>%
getElement(name = "id")

# order columns by decreasing percent of missing values
ordered_columns <- summary_plot_patients %>%
dplyr::select(-"id", -dplyr::all_of(parent_keys)) %>%
dplyr::summarise(
column = create_cols_labels(colnames(.)),
na_count = apply(., MARGIN = 2, FUN = sum),
na_percent = na_count / nrow(.) * 100
) %>%
dplyr::arrange(na_percent, dplyr::desc(column))

summary_plot_patients <- summary_plot_patients %>%
tidyr::gather("col", "isna", -"id", -dplyr::all_of(parent_keys)) %>%
dplyr::mutate(col = create_cols_labels(col))
})
substitute(
expr = {
summary_plot_patients <- ANL[, c(parent_keys, analysis_vars)] %>%
dplyr::group_by_at(parent_keys) %>%
dplyr::mutate(id = dplyr::cur_group_id()) %>%
dplyr::ungroup() %>%
dplyr::group_by_at(c(parent_keys, "id")) %>%
dplyr::summarise_all(anyNA) %>%
dplyr::ungroup()

# order subjects by decreasing number of missing and then by
# missingness pattern (defined using sha1)
order_subjects <- summary_plot_patients %>%
dplyr::select(-"id", -dplyr::all_of(parent_keys)) %>%
dplyr::transmute(
id = dplyr::row_number(),
number_NA = apply(., 1, sum),
sha = apply(., 1, hashing_function)
) %>%
dplyr::arrange(dplyr::desc(number_NA), sha) %>%
getElement(name = "id")

# order columns by decreasing percent of missing values
ordered_columns <- summary_plot_patients %>%
dplyr::select(-"id", -dplyr::all_of(parent_keys)) %>%
dplyr::summarise(
column = create_cols_labels(colnames(.)),
na_count = apply(., MARGIN = 2, FUN = sum),
na_percent = na_count / nrow(.) * 100
) %>%
dplyr::arrange(na_percent, dplyr::desc(column))

summary_plot_patients <- summary_plot_patients %>%
tidyr::gather("col", "isna", -"id", -dplyr::all_of(parent_keys)) %>%
dplyr::mutate(col = create_cols_labels(col))
},
env = list(hashing_function = hashing_function)
)
) %>%
teal.code::eval_code(
substitute(
Expand Down
5 changes: 0 additions & 5 deletions R/tm_t_crosstable.R
Original file line number Diff line number Diff line change
Expand Up @@ -140,11 +140,6 @@ tm_t_crosstable <- function(label = "Cross Table",
basic_table_args = teal.widgets::basic_table_args()) {
message("Initializing tm_t_crosstable")

# Requires Suggested packages
if (!requireNamespace("rtables", quietly = TRUE)) {
stop("Cannot load rtables - please install the package or restart your session.")
}

# Normalize the parameters
if (inherits(x, "data_extract_spec")) x <- list(x)
if (inherits(y, "data_extract_spec")) y <- list(y)
Expand Down
12 changes: 0 additions & 12 deletions R/tm_variable_browser.R
Original file line number Diff line number Diff line change
Expand Up @@ -78,17 +78,6 @@ tm_variable_browser <- function(label = "Variable Browser",
ggplot2_args = teal.widgets::ggplot2_args()) {
message("Initializing tm_variable_browser")

# Requires Suggested packages
if (!requireNamespace("sparkline", quietly = TRUE)) {
stop("Cannot load sparkline - please install the package or restart your session.")
}
if (!requireNamespace("htmlwidgets", quietly = TRUE)) {
stop("Cannot load htmlwidgets - please install the package or restart your session.")
}
if (!requireNamespace("jsonlite", quietly = TRUE)) {
stop("Cannot load jsonlite - please install the package or restart your session.")
}

# Start of assertions
checkmate::assert_string(label)
checkmate::assert_character(datasets_selected)
Expand Down Expand Up @@ -1280,7 +1269,6 @@ create_sparklines.default <- function(arr, width = 150, ...) {
as.character(tags$code("unsupported variable type", class = "text-blue"))
}


custom_sparkline_formatter <- function(labels, counts) {
htmlwidgets::JS(
sprintf(
Expand Down
4 changes: 3 additions & 1 deletion tests/testthat/setup-logger.R
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
logger::log_appender(function(...) NULL, namespace = "teal.modules.general")
if (requireNamespace("logger", quietly = TRUE)) {
logger::log_appender(function(...) NULL, namespace = "teal.modules.general")
}
1 change: 1 addition & 0 deletions tests/testthat/test-examples.R
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ for (i in rd_files()) {
paste0("example-", basename(i)),
{
skip_if_too_deep(5)
testthat::skip_if_not_installed("pkgload")
if (basename(i) %in% strict_exceptions) {
op <- options()
withr::local_options(opts_partial_match_old)
Expand Down
1 change: 1 addition & 0 deletions tests/testthat/test-shinytest2-tm_data_table.R
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ test_that("e2e - tm_data_table: Verify checkbox displayed over data table", {
})

test_that("e2e - tm_data_table: Verify module displays data table", {
testthat::skip_if_not_installed("rvest")
Copy link
Contributor

@pawelru pawelru Aug 15, 2024

Choose a reason for hiding this comment

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

I have recently pushed changes into the teal due to noSuggest check and the class definition looks like this:
https://github.com/insightsengineering/teal/blob/7eac4d58a5373aa511f363a2f259d850749501f7/R/TealAppDriver.R#L10-L20

Just thinking loud now, would it help if we would have the following instead?

    if (!requireNamespace("shinytest2", quietly = TRUE)) {
      if (requireNamespace("testthat", quietly = TRUE) && getNamespace("testthat")$is_testing()) {  
       getNamespace("testthat")$skip("shinytest2 is not installed")
      } else {
        stop("Please install 'shinytest2' package to use this class.")
      }
    }

My motivation is to avoid a lot of skip statements in the upstream packages like here.

skip_if_too_deep(5)
app_driver <- app_driver_tm_data_table()

Expand Down
3 changes: 3 additions & 0 deletions tests/testthat/test-shinytest2-tm_file_viewer.R
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ app_driver_tm_file_viewer <- function() {
}

test_that("e2e - tm_file_viewer: Initializes without errors and shows files tree specified in input_path argument", {
testthat::skip_if_not_installed("rvest")
skip_if_too_deep(5)
app_driver <- app_driver_tm_file_viewer()

Expand All @@ -42,6 +43,7 @@ test_that("e2e - tm_file_viewer: Initializes without errors and shows files tree
})

test_that("e2e - tm_file_viewer: Shows selected image file", {
testthat::skip_if_not_installed("rvest")
skip_if_too_deep(5)
app_driver <- app_driver_tm_file_viewer()

Expand All @@ -58,6 +60,7 @@ test_that("e2e - tm_file_viewer: Shows selected image file", {
})

test_that("e2e - tm_file_viewer: Shows selected text file", {
testthat::skip_if_not_installed("rvest")
skip_if_too_deep(5)
app_driver <- app_driver_tm_file_viewer()

Expand Down
1 change: 1 addition & 0 deletions tests/testthat/test-shinytest2-tm_misssing_data.R
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ test_that("e2e - tm_missing_data: Validate functionality and UI response for 'By
})

test_that("e2e - tm_missing_data: Validate 'By Variable Levels' table values", {
testthat::skip_if_not_installed("rvest")
skip_if_too_deep(5)
app_driver <- app_driver_tm_missing_data()

Expand Down
Loading