From 862535ecb03c2c259633e36e8245e924a5a60cf4 Mon Sep 17 00:00:00 2001 From: RasmusSkytte Date: Thu, 21 Dec 2023 10:52:53 +0000 Subject: [PATCH 01/14] chore: created local '.github/workflows/synchronise-files.yaml' from remote '.github/workflows/synchronise-files.yaml' --- .github/workflows/synchronise-files.yaml | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100644 .github/workflows/synchronise-files.yaml diff --git a/.github/workflows/synchronise-files.yaml b/.github/workflows/synchronise-files.yaml new file mode 100644 index 00000000..aa967978 --- /dev/null +++ b/.github/workflows/synchronise-files.yaml @@ -0,0 +1,21 @@ +on: + push: + branches: + - main + workflow_dispatch: + +jobs: + sync: + name: 🔄 Sync diseasyverse files + runs-on: ubuntu-latest + permissions: write-all + steps: + - name: Checkout Repository + uses: actions/checkout@master + - name: Run GitHub File Sync + uses: BetaHuhn/repo-file-sync-action@v1 + with: + GH_PAT: ${{ secrets.GH_PAT }} + CONFIG_PATH: .github/sync.yaml + COMMIT_PREFIX: "chore: " + PR_BODY: Automatically synchronise files between diseasyverse repositories From d96add4605309cd3522aedbab9ef1d4e5ff37a24 Mon Sep 17 00:00:00 2001 From: RasmusSkytte Date: Thu, 21 Dec 2023 10:52:53 +0000 Subject: [PATCH 02/14] chore: created local '.github/sync.yaml' from remote '.github/sync.yaml' --- .github/sync.yaml | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 .github/sync.yaml diff --git a/.github/sync.yaml b/.github/sync.yaml new file mode 100644 index 00000000..ba26da91 --- /dev/null +++ b/.github/sync.yaml @@ -0,0 +1,32 @@ +ssi-dk/diseasystore: + - .github/workflows/synchronise-files.yaml + - .github/sync.yaml + + - R/0_linters.R + - tests/testthat/test-0_linters.R + + - R/0_R6_utils.R + - tests/testthat/test-0_R6_utils.R + + - R/0_documentation.R + - tests/testthat/test-0_documentation.R + + - tests/testthat/test-0_return.R + - tests/testthat/test-0_examples.R + + +ssi-dk/diseasy: + - .github/workflows/synchronise-files.yaml + - .github/sync.yaml + + - R/0_linters.R + - tests/testthat/test-0_linters.R + + - R/0_R6_utils.R + - tests/testthat/test-0_R6_utils.R + + - R/0_documentation.R + - tests/testthat/test-0_documentation.R + + - tests/testthat/test-0_return.R + - tests/testthat/test-0_examples.R From 4a89350b195eb44b27379814f05c3314b11530ea Mon Sep 17 00:00:00 2001 From: RasmusSkytte Date: Thu, 21 Dec 2023 10:52:53 +0000 Subject: [PATCH 03/14] chore: synced local 'R/0_linters.R' with remote 'R/0_linters.R' --- R/0_linters.R | 128 ++++++++++++++++++++++---------------------------- 1 file changed, 55 insertions(+), 73 deletions(-) diff --git a/R/0_linters.R b/R/0_linters.R index 65f64201..a1007846 100644 --- a/R/0_linters.R +++ b/R/0_linters.R @@ -1,31 +1,24 @@ #' The custom linters of `diseasy` -#' @name diseasy_linters -#' @description -#' nolint_position_linter: Check that the `nolint:` statements occur after the character limit #' -#' @param length maximum line length allowed. Default is 80L (Hollerith limit). -#' @returns A list of `lintr::Lint` +#' @name diseasy_linters #' @examples -#' ## nolint_position_linter -#' # will produce lints -#' lintr::lint( -#' text = paste0(strrep("x", 15L), "# nolint: object_name_linter"), -#' linters = c(nolint_position_linter(length = 20L), lintr::object_name_linter()) -#' ) -#' -#' # okay -#' lintr::lint( -#' text = paste0(strrep("x", 20L), "# nolint: object_name_linter"), -#' linters = c(nolint_position_linter(length = 20L), lintr::object_name_linter()) -#' ) -#' -#' @seealso -#' - [lintr::linters] for a complete list of linters available in lintr. -#' - +#' diseasy_code_linters() +#' @return A list of linters #' @export +diseasy_code_linters <- function() { + linters <- list( + non_ascii_linter(), + todo_linter() + ) + + return(linters) +} + + +#' @rdname diseasy_linters #' @importFrom rlang .data -nolint_position_linter <- function(length = 80L) { - general_msg <- paste("`nolint:` statements start at", length + 1, "characters.") +non_ascii_linter <- function() { + general_msg <- paste("Code should not contain non-ASCII characters") lintr::Linter( function(source_expression) { @@ -35,27 +28,28 @@ nolint_position_linter <- function(length = 80L) { return(list()) } - nolint_info <- source_expression$content |> - stringr::str_locate_all(stringr::regex(r"{# *nolint}", ignore_case = TRUE)) + detection_info <- source_expression$file_lines |> + stringr::str_locate_all(stringr::regex(r"{[^\x00-\x7f]}", ignore_case = TRUE)) - nolint_info <- purrr::map2( - nolint_info, - seq_along(nolint_info), + detection_info <- purrr::map2( + detection_info, + seq_along(detection_info), ~ dplyr::mutate(as.data.frame(.x), line_number = .y) - ) |> + ) + + detection_info <- detection_info |> purrr::reduce(rbind) |> - dplyr::filter(!is.na(.data$start)) |> - dplyr::filter(.data$start <= length) + dplyr::filter(!is.na(.data$start)) purrr::pmap( - nolint_info, + detection_info, \(start, end, line_number) { lintr::Lint( filename = source_expression$filename, line_number = line_number, column_number = start, type = "style", - message = paste(general_msg, "This statement starts at", start, "characters"), + message = paste(general_msg, "non-ASCII character found"), line = source_expression$file_lines[line_number], ranges = list(c(start, end)) ) @@ -66,29 +60,9 @@ nolint_position_linter <- function(length = 80L) { } -#' @name diseasy_linters -#' @description -#' nolint_line_length_linter: Check that lines adhere to a given character limit, ignoring `nolint` statements -#' -#' @param length maximum line length allowed. Default is 80L (Hollerith limit). -#' @examples -#' ## nolint_line_length_linter -#' # will produce lints -#' lintr::lint( -#' text = paste0(strrep("x", 25L), "# nolint: object_name_linter."), -#' linters = c(nolint_line_length_linter(length = 20L), lintr::object_name_linter()) -#' ) -#' -#' # okay -#' lintr::lint( -#' text = paste0(strrep("x", 20L), "# nolint: object_name_linter."), -#' linters = c(nolint_line_length_linter(length = 20L), lintr::object_name_linter()) -#' ) -#' -#' @export -#' @importFrom rlang .data -nolint_line_length_linter <- function(length = 80L) { - general_msg <- paste("Lines should not be more than", length, "characters.") +#' @rdname diseasy_linters +todo_linter <- function() { + general_msg <- paste("`TODO` statements should not be kept in code base:") lintr::Linter( function(source_expression) { @@ -98,23 +72,31 @@ nolint_line_length_linter <- function(length = 80L) { return(list()) } - nolint_regex <- r"{# ?no(lint|cov) ?(start|end)?:?.*}" - - file_lines_nolint_excluded <- source_expression$file_lines |> - purrr::map_chr(\(s) stringr::str_remove(s, nolint_regex)) - - line_lengths <- nchar(file_lines_nolint_excluded) - long_lines <- which(line_lengths > length) - Map(function(long_line, line_length) { - lintr::Lint( - filename = source_expression$filename, - line_number = long_line, - column_number = length + 1L, type = "style", - message = paste(general_msg, "This line is", line_length, "characters."), - line = source_expression$file_lines[long_line], - ranges = list(c(1L, line_length)) - ) - }, long_lines, line_lengths[long_lines]) + todo_info <- source_expression$file_lines |> + stringr::str_locate_all(stringr::regex(r"{(?<=\s|^)todos?:?(?=\s|$)}", ignore_case = TRUE)) + + todo_info <- purrr::map2( + todo_info, + seq_along(todo_info), + ~ dplyr::mutate(as.data.frame(.x), line_number = .y) + ) |> + purrr::reduce(rbind) |> + dplyr::filter(!is.na(.data$start)) + + purrr::pmap( + todo_info, + \(start, end, line_number) { + lintr::Lint( + filename = source_expression$filename, + line_number = line_number, + column_number = start, + type = "style", + message = paste(general_msg, "`TODO` statement found"), + line = source_expression$file_lines[line_number], + ranges = list(c(start, end)) + ) + } + ) } ) } From 35610af727eccbba7d58ac37e7eed82c342938ad Mon Sep 17 00:00:00 2001 From: RasmusSkytte Date: Thu, 21 Dec 2023 10:52:53 +0000 Subject: [PATCH 04/14] chore: synced local 'tests/testthat/test-0_linters.R' with remote 'tests/testthat/test-0_linters.R' --- tests/testthat/test-0_linters.R | 37 ++++++++------------------------- 1 file changed, 9 insertions(+), 28 deletions(-) diff --git a/tests/testthat/test-0_linters.R b/tests/testthat/test-0_linters.R index db8318e3..931cc61f 100644 --- a/tests/testthat/test-0_linters.R +++ b/tests/testthat/test-0_linters.R @@ -1,28 +1,9 @@ -test_that("nolint_position_linter works", { # nolint start: nolint_position_linter - lintr::expect_lint( - paste0(strrep("x", 5), "# nolint: object_name_linter."), - list("column_number" = 6, "type" = "style"), - c(nolint_position_linter(length = 20L), lintr::object_name_linter()) - ) - - lintr::expect_lint( - paste0(strrep("x", 20), "# nolint: object_name_linter."), - NULL, - c(nolint_position_linter(length = 20L), lintr::object_name_linter()) - ) -}) # nolint end - - -test_that("nolint_line_length_linter works", { # nolint start: nolint_position_linter - lintr::expect_lint( - paste0(strrep("x", 5), "# nolint: object_name_linter."), - list("column_number" = 5, "type" = "style"), - c(nolint_line_length_linter(length = 4L), lintr::object_name_linter()) - ) - - lintr::expect_lint( - paste0(strrep("x", 5), "# nolint: object_name_linter."), - NULL, - c(nolint_line_length_linter(length = 5L), lintr::object_name_linter()) - ) -}) # nolint end +test_that("non_ascii_linter works", { + lintr::expect_lint("æ", list("line_number" = 1, "type" = "style"), non_ascii_linter()) # nolint start: non_ascii_linter + lintr::expect_lint("\nø", list("line_number" = 2, "type" = "style"), non_ascii_linter()) + lintr::expect_lint("\n\nå", list("line_number" = 3, "type" = "style"), non_ascii_linter()) + lintr::expect_lint("æ", list("line_number" = 1, "type" = "style"), non_ascii_linter()) # nolint end + + lintr::expect_lint("xxx", NULL, non_ascii_linter()) + lintr::expect_lint("abc", NULL, non_ascii_linter()) +}) From 7f9fad5eaa182ba77d81d3997717e0fc76bb9f4d Mon Sep 17 00:00:00 2001 From: RasmusSkytte Date: Thu, 21 Dec 2023 10:52:53 +0000 Subject: [PATCH 05/14] chore: synced local 'R/0_R6_utils.R' with remote 'R/0_R6_utils.R' --- R/0_R6_utils.R | 66 +++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 54 insertions(+), 12 deletions(-) diff --git a/R/0_R6_utils.R b/R/0_R6_utils.R index 4a3aba43..2dcfd4bc 100644 --- a/R/0_R6_utils.R +++ b/R/0_R6_utils.R @@ -21,13 +21,53 @@ read_only_error <- function(field) { #' cat printing with default new line -#' @param ... The normal input to cat -#' @param file Path of an output file to append the output to -#' @param sep The separator given to cat +#' @param ... +#' The normal input to cat +#' @param file +#' Path of an output file to append the output to +#' @param sep (`character`)\cr +#' The separator given to cat +#' @param max_width (`numeric`)\cr +#' The maximum number of characters to print before inserting a newline. +#' NULL (default) does not break up lines. #' @noRd -printr <- function(..., file = nullfile(), sep = "") { - withr::local_output_sink(new = file, split = TRUE, append = TRUE) - cat(..., "\n", sep = sep) +printr <- function(..., file = nullfile(), sep = "", max_width = NULL) { + sink(file = file, split = TRUE, append = TRUE, type = "output") + + print_string <- paste(..., sep = sep) + + # If a width limit is set, we iteratively determine the words that exceed the limit and + # insert a newline + if (!is.null(max_width)) { + + # Get the current state of the string + split_string <- stringr::str_split_1(print_string, "\n") + segment_lengths <- purrr::map_dbl(split_string, ~ length(stringr::str_split_1(., " "))) + + # While segments contain more than one word and is longer than max_width, split these segments + while (any(nchar(split_string) > max_width & segment_lengths > 1)) { + split_string <- split_string |> + purrr::map_if( + ~ nchar(.) > max_width, + ~ { + break_locations <- stringr::str_locate_all(., " |$")[[1]][, 1] + split_width <- break_locations[max(which((break_locations - 1) < max_width))] + + stringr::str_replace(., paste0("(?<=^.{", split_width - 1, "})(\\w*) "), "\\1\n") + } + ) |> + stringr::str_split("\n") |> + purrr::reduce(c) + + segment_lengths <- purrr::map_dbl(split_string, ~ length(stringr::str_split_1(., " "))) + } + + # Collapse segments with the newline + print_string <- paste(split_string, collapse = "\n") + } + + cat(print_string, "\n", sep = sep) + sink() } @@ -44,7 +84,7 @@ printr <- function(..., file = nullfile(), sep = "") { #' # Retrieve DiseasystoreGoogleCovid19 specific option for source conn #' diseasyoption("source_conn", "DiseasystoreGoogleCovid19") #' -#' # Try to retrieve specific option for source conn for a non existent / un-configured diseasystore +#' # Try to retrieve specific option for source conn for a non existant / unconfigured diseasystore #' diseasyoption("source_conn", "DiseasystoreNonExistent") # Returns default source_conn #' @export diseasyoption <- function(option, class = "DiseasystoreBase") { @@ -53,7 +93,7 @@ diseasyoption <- function(option, class = "DiseasystoreBase") { class <- base::class(class)[1] } - base_class <- stringr::str_extract(class, r"{^([A-Z][a-z]*)}") |> # nolint: object_usage_linter + base_class <- stringr::str_extract(class, r"{^([A-Z][a-z]*)}") |> # nolint: object_usage_linter stringr::str_to_lower() list(class, NULL) |> @@ -61,7 +101,7 @@ diseasyoption <- function(option, class = "DiseasystoreBase") { purrr::map(getOption) |> purrr::map(unlist) |> purrr::keep(purrr::negate(is.null)) |> - purrr::discard(~ identical(., "")) |> + purrr::discard(~ is.character(.) && . == "") |> purrr::pluck(1) } @@ -78,7 +118,7 @@ parse_diseasyconn <- function(conn, type = "source_conn") { checkmate::assert( checkmate::check_function(conn, null.ok = TRUE), checkmate::check_class(conn, "DBIConnection", null.ok = TRUE), - checkmate::check_character(conn, null.ok = TRUE), + checkmate::check_character(conn, len = 1, null.ok = TRUE), add = coll ) checkmate::assert_choice(type, c("source_conn", "target_conn"), add = coll) @@ -87,7 +127,7 @@ parse_diseasyconn <- function(conn, type = "source_conn") { if (is.null(conn)) { return(conn) } else if (is.function(conn)) { - tryCatch(conn <- conn(), # nolint: implicit_assignment_linter + tryCatch(conn <- conn(), error = \(e) stop("`conn` could not be parsed!")) return(conn) } else if (type == "target_conn" && inherits(conn, "DBIConnection")) { @@ -114,7 +154,9 @@ parse_diseasyconn <- function(conn, type = "source_conn") { #' t %.% a # 1 #' #' t$c # NULL -#' try(t %.% c) # Gives error since "c" does not exist in "t" +#' \dontrun{ +#' t %.% c # ERROR a not found in t +#' } #' @export `%.%` <- function(env, field) { field_name <- as.character(substitute(field)) From 73d4b5a6db8979fb539cd21a97ea439f7522e8c0 Mon Sep 17 00:00:00 2001 From: RasmusSkytte Date: Thu, 21 Dec 2023 10:52:53 +0000 Subject: [PATCH 06/14] chore: synced local 'tests/testthat/test-0_R6_utils.R' with remote 'tests/testthat/test-0_R6_utils.R' --- tests/testthat/test-0_R6_utils.R | 85 +++++++++----------------------- 1 file changed, 24 insertions(+), 61 deletions(-) diff --git a/tests/testthat/test-0_R6_utils.R b/tests/testthat/test-0_R6_utils.R index ee300437..0fcecc22 100644 --- a/tests/testthat/test-0_R6_utils.R +++ b/tests/testthat/test-0_R6_utils.R @@ -1,60 +1,29 @@ -test_that("printr works", { - - # Check that printr works without file printing - # 1) - checkmate::expect_character(capture.output(printr("test string")), pattern = r"{test string}") - # 2) - checkmate::expect_character(capture.output(printr("test1", "test2")), pattern = r"{test1test2}") - # 3) - checkmate::expect_character(capture.output(printr("test1", "test2", sep = " ")), pattern = r"{test1 test2}") - - # Check that printr works with file printing - test_file <- tempfile() - - # 1) - checkmate::expect_character(capture.output(printr("test string", file = test_file)), - pattern = r"{test string}") - checkmate::expect_character(readLines(test_file), - pattern = r"{test string}") - file.remove(test_file) - - - # 2) - checkmate::expect_character(capture.output(printr("test1", "test2", file = test_file)), - pattern = r"{test1test2}") - checkmate::expect_character(readLines(test_file), - pattern = r"{test1test2}") - file.remove(test_file) - - - # 3) - checkmate::expect_character(capture.output(printr("test1", "test2", file = test_file, sep = " ")), - pattern = r"{test1 test2}") - checkmate::expect_character(readLines(test_file), - pattern = r"{test1 test2}") - file.remove(test_file) -}) - - test_that("diseasyoption works", { + # Store the current options + opts <- options( + "diseasystore.target_schema" = "", + "diseasystore.DiseasystoreGoogleCovid19.target_schema" = "" + ) + # Check that diseasyoption works for default values - expect_null(diseasyoption("non_existent_option")) + expect_equal(diseasyoption("target_schema"), NULL) - withr::local_options("diseasystore.target_schema" = target_schema_1) - expect_identical(diseasyoption("target_schema"), target_schema_1) + options("diseasystore.target_schema" = "target_schema_1") + expect_equal(diseasyoption("target_schema"), "target_schema_1") # Check that it works for child classes ds <- DiseasystoreGoogleCovid19$new(target_conn = DBI::dbConnect(RSQLite::SQLite())) - expect_identical(diseasyoption("target_schema", "DiseasystoreGoogleCovid19"), target_schema_1) - expect_identical(diseasyoption("target_schema", ds), target_schema_1) + expect_equal(diseasyoption("target_schema", "DiseasystoreGoogleCovid19"), "target_schema_1") + expect_equal(diseasyoption("target_schema", ds), "target_schema_1") - withr::local_options("diseasystore.DiseasystoreGoogleCovid19.target_schema" = target_schema_2) - expect_identical(diseasyoption("target_schema", "DiseasystoreGoogleCovid19"), target_schema_2) - expect_identical(diseasyoption("target_schema", ds), target_schema_2) + options("diseasystore.DiseasystoreGoogleCovid19.target_schema" = "target_schema_2") + expect_equal(diseasyoption("target_schema", "DiseasystoreGoogleCovid19"), "target_schema_2") + expect_equal(diseasyoption("target_schema", ds), "target_schema_2") - rm(ds) - invisible(gc()) + # Reset options + options(opts) + rm(opts) }) @@ -98,7 +67,7 @@ test_that("parse_diseasyconn works", { null_conn <- NULL # Test inputs for source_conn - conn <- expect_no_condition(parse_diseasyconn(valid_function_conn, type = "source_conn")) + expect_no_condition(conn <- parse_diseasyconn(valid_function_conn, type = "source_conn")) checkmate::expect_class(conn, "DBIConnection") DBI::dbDisconnect(conn) @@ -107,21 +76,19 @@ test_that("parse_diseasyconn works", { class = "simpleError", regexp = "`conn` could not be parsed!") - conn <- expect_no_condition(parse_diseasyconn(valid_dbi_conn, type = "source_conn")) + expect_no_condition(conn <- parse_diseasyconn(valid_dbi_conn, type = "source_conn")) expect_identical(conn, valid_dbi_conn) - checkmate::expect_class(conn, "DBIConnection") - - conn <- expect_no_condition(parse_diseasyconn(valid_str_conn, type = "source_conn")) + expect_no_condition(conn <- parse_diseasyconn(valid_str_conn, type = "source_conn")) expect_identical(conn, valid_str_conn) - conn <- expect_no_condition(parse_diseasyconn(null_conn, type = "source_conn")) + expect_no_condition(conn <- parse_diseasyconn(null_conn, type = "source_conn")) expect_null(conn) # Test inputs for target_conn - conn <- expect_no_condition(parse_diseasyconn(valid_function_conn, type = "target_conn")) + expect_no_condition(conn <- parse_diseasyconn(valid_function_conn, type = "target_conn")) checkmate::expect_class(conn, "DBIConnection") DBI::dbDisconnect(conn) @@ -130,18 +97,14 @@ test_that("parse_diseasyconn works", { class = "simpleError", regexp = "`conn` could not be parsed!") - conn <- expect_no_condition(parse_diseasyconn(valid_dbi_conn, type = "target_conn")) + expect_no_condition(conn <- parse_diseasyconn(valid_dbi_conn, type = "target_conn")) expect_identical(conn, valid_dbi_conn) - checkmate::expect_class(conn, "DBIConnection") expect_error(parse_diseasyconn(valid_str_conn, type = "target_conn"), class = "simpleError", regexp = "`conn` could not be parsed!") - conn <- expect_no_condition(parse_diseasyconn(null_conn, type = "target_conn")) + expect_no_condition(conn <- parse_diseasyconn(null_conn, type = "target_conn")) expect_null(conn) - - # Test clean up - DBI::dbDisconnect(valid_dbi_conn) }) From dcfa9b1999d10e65d9f7b1b948125e4be2108bc1 Mon Sep 17 00:00:00 2001 From: RasmusSkytte Date: Thu, 21 Dec 2023 10:52:53 +0000 Subject: [PATCH 07/14] chore: synced local 'R/0_documentation.R' with remote 'R/0_documentation.R' --- R/0_documentation.R | 59 +++++++++++++++++++++++++++++++++++++-------- 1 file changed, 49 insertions(+), 10 deletions(-) diff --git a/R/0_documentation.R b/R/0_documentation.R index 427e3c45..107a0a58 100644 --- a/R/0_documentation.R +++ b/R/0_documentation.R @@ -1,13 +1,21 @@ -rd_aggregation <- function(type = "param") { +rd_activity_units <- function(type = "param") { + checkmate::assert_choice(type, c("param", "field")) + paste("(`list(list())`)\\cr", + "A nested list of all possible 'units' of activity that can be opened or closed.", + ifelse(type == "field", " Read only.", "")) +} + + +rd_stratification <- function(type = "param") { checkmate::assert_choice(type, c("param", "field")) paste("(`list`(`quosures`))\\cr", "Default NULL.", - "If given, expressions in aggregation evaluated to give the aggregation level.", + "If given, expressions in stratification evaluated to give the stratification level.", ifelse(type == "field", " Read only.", "")) } -rd_diseasystore_label <- function(type = "param") { +rd_diseasystore <- function(type = "param") { checkmate::assert_choice(type, c("param", "field")) paste("(`character`)\\cr", "A character string that controls which feature store to get data from.", @@ -15,6 +23,22 @@ rd_diseasystore_label <- function(type = "param") { } +rd_contact_basis <- function(type = "param") { + checkmate::assert_choice(type, c("param", "field")) + paste("(`list(list())`)\\cr", + "A nested list with all the needed information for the contact_basis\\cr", + "* `counts` contains the age stratified contact counts across the arenas of the basis", + " (e.g. 'work', 'home', 'school', 'other')\\cr", + "* `proportion` contains a list of the proportion of population in each age-group\\cr", + "* `demography` contains a `data.frame` with the columns\\cr", + " * `age` (`integer()`) 1-year age group\\cr", + " * `population` (`numeric()`) size of population in age group\\cr", + " * `proportion` (`numeric()`) proportion of total population in age group\\cr", + "* `description` contains information about the source of the contact basis.", + ifelse(type == "field", " Read only.", "")) +} + + rd_observable <- function(type = "param") { checkmate::assert_choice(type, c("param", "field")) paste("(`character`)\\cr", @@ -53,7 +77,7 @@ rd_scale <- function(type = "param") { rd_source_conn <- function(type = "param") { checkmate::assert_choice(type, c("param", "field")) - paste("(`DBIConnection` or `file path`)\\cr", + paste("source_conn\\cr", "Used to specify where data is located.", ifelse(type == "field", " Read only.", ""), "Can be `DBIConnection` or file path depending on the `diseasystore`.") @@ -96,7 +120,7 @@ rd_start_date <- function(type = "param") { rd_slice_ts <- function(type = "param") { checkmate::assert_choice(type, c("param", "field")) paste("(`Date` or `character`)\\cr", - "Date to slice the database on (used if source_conn is a database).", + "Date or timestamp (parseable by `as.POSIXct`) to slice the database on (used if source_conn is a database).", ifelse(type == "field", " Read only.", "")) } @@ -111,8 +135,8 @@ rd_end_date <- function(type = "param") { rd_.data <- function(type = "param") { # nolint: object_name_linter checkmate::assert_choice(type, c("param", "field")) - paste("(`any`)\\cr", - "The data object to perform the operation on.", + paste(".data\\cr", + "The data object on which to perform the operation.", ifelse(type == "field", " Read only.", "")) } @@ -120,12 +144,12 @@ rd_.data <- function(type = "param") { rd_describe <- "Prints a human readable report of the internal state of the module." rd_get_results_description <- paste( - "The primary method used to request model results of a given observable at a given aggregation." + "The primary method used to request model results of a given observable at a given stratification" ) rd_get_results_return <- paste( - "A `tibble` [tibble::tibble] with predictions at the level specified by aggregation level.", - "In addition to aggregation columns, the output has the columns:\\cr", + "A `tibble` [tibble::tibble] with predictions at the level specified by stratification level.", + "In addition to stratification columns, the output has the columns:\\cr", " date (`Date`) specifying the date of the prediction\\cr", " realization_id (`character`) giving a unique id for each realization in the ensemble\\cr", " model (`character`) the name (classname) of the model used to provide the prediction." @@ -154,3 +178,18 @@ rd_training_length <- paste( "(`numeric`)\\cr", "The number of days that should be included in the training of the model." ) + + +rd_side_effects <- "NULL (called for side effects)" + + + +rd_age_cuts_lower <- paste( + "(`numeric`)\\cr", + "vector of ages defining the lower bound for each age group. If NULL (default), age groups of contact_basis is used." +) + +rd_activity_weights <- paste( + "(`numeric(4)`)\\cr", + "vector of weights for the four types of contacts. If NULL (default), no weighting is done." +) From dbff2bc229f419570ae96aa0389f33fdf3cf900a Mon Sep 17 00:00:00 2001 From: RasmusSkytte Date: Thu, 21 Dec 2023 10:52:53 +0000 Subject: [PATCH 08/14] chore: synced local 'tests/testthat/test-0_documentation.R' with remote 'tests/testthat/test-0_documentation.R' --- tests/testthat/test-0_documentation.R | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/testthat/test-0_documentation.R b/tests/testthat/test-0_documentation.R index 7f425cd0..cc98b77d 100644 --- a/tests/testthat/test-0_documentation.R +++ b/tests/testthat/test-0_documentation.R @@ -1,12 +1,12 @@ test_that("rd_templates works", { - pkg_objects <- ls(base::getNamespace("diseasystore")) + pkg_objects <- ls(base::getNamespace("diseasy")) rd_objects <- purrr::keep(pkg_objects, ~ startsWith(., "rd_")) - rd_functions <- rd_objects[purrr::map_lgl(rd_objects, ~ rlang::is_function(get(.)))] + rd_funs <- rd_objects[purrr::map_lgl(rd_objects, ~ rlang::is_function(get(.)))] for (type in c("field", "param")) { - for (rd_fun in rd_functions) { - str <- expect_no_condition(do.call(rd_fun, args = list(type = type))) + for (rd_fun in rd_funs) { + expect_no_condition(str <- do.call(rd_fun, args = list(type = type))) checkmate::expect_character(str) } } From 0fab43357ad95a693d1c7b3cbcf2e91d06bcdd84 Mon Sep 17 00:00:00 2001 From: RasmusSkytte Date: Thu, 21 Dec 2023 10:52:53 +0000 Subject: [PATCH 09/14] chore: synced local 'tests/testthat/test-0_return.R' with remote 'tests/testthat/test-0_return.R' --- tests/testthat/test-0_return.R | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/tests/testthat/test-0_return.R b/tests/testthat/test-0_return.R index b64bfb93..4564dd48 100644 --- a/tests/testthat/test-0_return.R +++ b/tests/testthat/test-0_return.R @@ -1,9 +1,23 @@ test_that(r"{.Rd files have \Value}", { - # Look for the source of .Rd files - help_dir <- system.file("help", package = testthat::testing_package()) - man_dir <- system.file("man", package = testthat::testing_package()) + # Load .Rd files based on environment + # When using R-CMD-Check the deployment is different from when using devtools::test(). + # Deployment on Github is also different from running R-CMD-Check locally + # Here we need to read directly from a .rdx database stored in the "help" folder + # If we are testing locally, we read from the "man" folder + # Note that `devtools::test()`, `devtools::check()` and GitHub workflows all have different + # folder structures during testing, so we need to account for these differences + + # Platform independent regex search to find root folder + path_regex <- stringr::str_replace(r"{(.*\/\w*(.Rcheck)?)\/.*(?=\/testthat)}", "/", .Platform$file.sep) + pkg_path <- stringr::str_extract(getwd(), path_regex, group = 1) + # If path contains .Rcheck, we need to add the package name to the path + pkg_path <- stringr::str_replace(pkg_path, r"{(\w*)(\.Rcheck)}", paste0("\\1\\2", .Platform$file.sep, "\\1")) + + # Look for the source of .Rd files + help_dir <- file.path(pkg_path, "help") + man_dir <- file.path(pkg_path, "man") if (checkmate::test_directory_exists(help_dir)) { @@ -11,7 +25,8 @@ test_that(r"{.Rd files have \Value}", { rd_envir <- new.env() lazyLoad(stringr::str_remove(rdx_file, ".rdx$"), envir = rd_envir) rd_names <- ls(rd_envir) - rd_files <- purrr::map(rd_names, ~ as.character(eval(purrr::pluck(rd_envir, .)))) + rd_files <- purrr::map(rd_names, ~ as.character(eval(purrr::pluck(rd_envir, .)))) |> + purrr::map(~ paste(., collapse = "")) names(rd_files) <- paste0(rd_names, ".Rd") } else if (checkmate::test_directory_exists(man_dir)) { @@ -30,9 +45,12 @@ test_that(r"{.Rd files have \Value}", { # Skip the "*-package.Rd" file rd_files <- rd_files[!stringr::str_detect(names(rd_files), "-package.[Rr][Dd]$")] + # Skip the "data" files + rd_files <- purrr::discard(rd_files, ~ any(stringr::str_detect(., r"{\\keyword\{data\}}"))) + # Check renaming for (rd_id in seq_along(rd_files)) { - has_value <- any(stringr::str_detect(rd_files[[rd_id]], stringr::fixed(r"{\value}"))) + has_value <- any(stringr::str_detect(rd_files[[rd_id]], r"{\\value}")) expect_true(has_value, label = paste("File:", names(rd_files)[[rd_id]])) } }) From bf2f23a61bdf59132619a0445af1fc18e8e0fbda Mon Sep 17 00:00:00 2001 From: RasmusSkytte Date: Thu, 21 Dec 2023 10:52:53 +0000 Subject: [PATCH 10/14] chore: synced local 'tests/testthat/test-0_examples.R' with remote 'tests/testthat/test-0_examples.R' --- tests/testthat/test-0_examples.R | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/tests/testthat/test-0_examples.R b/tests/testthat/test-0_examples.R index 89c54c1e..abe1d0e2 100644 --- a/tests/testthat/test-0_examples.R +++ b/tests/testthat/test-0_examples.R @@ -1,8 +1,23 @@ test_that(r"{.Rd files have \examples}", { + # Load .Rd files based on environment + # When using R-CMD-Check the deployment is different from when using devtools::test(). + # Deployment on Github is also different from running R-CMD-Check locally + # Here we need to read directly from a .rdx database stored in the "help" folder + # If we are testing locally, we read from the "man" folder + # Note that `devtools::test()`, `devtools::check()` and GitHub workflows all have different + # folder structures during testing, so we need to account for these differences + + # Platform independent regex search to find root folder + path_regex <- stringr::str_replace(r"{(.*\/\w*(.Rcheck)?)\/.*(?=\/testthat)}", "/", .Platform$file.sep) + pkg_path <- stringr::str_extract(getwd(), path_regex, group = 1) + + # If path contains .Rcheck, we need to add the package name to the path + pkg_path <- stringr::str_replace(pkg_path, r"{(\w*)(\.Rcheck)}", paste0("\\1\\2", .Platform$file.sep, "\\1")) + # Look for the source of .Rd files - help_dir <- system.file("help", package = testthat::testing_package()) - man_dir <- system.file("man", package = testthat::testing_package()) + help_dir <- file.path(pkg_path, "help") + man_dir <- file.path(pkg_path, "man") if (checkmate::test_directory_exists(help_dir)) { @@ -10,7 +25,8 @@ test_that(r"{.Rd files have \examples}", { rd_envir <- new.env() lazyLoad(stringr::str_remove(rdx_file, ".rdx$"), envir = rd_envir) rd_names <- ls(rd_envir) - rd_files <- purrr::map(rd_names, ~ as.character(eval(purrr::pluck(rd_envir, .)))) + rd_files <- purrr::map(rd_names, ~ as.character(eval(purrr::pluck(rd_envir, .)))) |> + purrr::map(~ paste(., collapse = "")) names(rd_files) <- paste0(rd_names, ".Rd") } else if (checkmate::test_directory_exists(man_dir)) { @@ -29,9 +45,12 @@ test_that(r"{.Rd files have \examples}", { # Skip the "*-package.Rd" file rd_files <- rd_files[!stringr::str_detect(names(rd_files), "-package.[Rr][Dd]$")] + # Skip the "data" files + rd_files <- purrr::discard(rd_files, ~ any(stringr::str_detect(., r"{\\keyword\{data\}}"))) + # Check renaming for (rd_id in seq_along(rd_files)) { - has_example <- any(stringr::str_detect(rd_files[[rd_id]], stringr::fixed(r"{\example}"))) + has_example <- any(stringr::str_detect(rd_files[[rd_id]], r"{\\example}")) expect_true(has_example, label = paste("File:", names(rd_files)[[rd_id]])) } }) From 1500a185bbacd166dbd099e76ccd1274b50d7b5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rasmus=20Skytte=20Randl=C3=B8v?= Date: Thu, 21 Dec 2023 13:48:33 +0100 Subject: [PATCH 11/14] chore: resolve merge conflicts preferring newest code --- .github/sync.yaml | 6 +- .lintr | 3 +- DESCRIPTION | 1 + NAMESPACE | 2 - R/0_R6_utils.R | 14 +- R/0_documentation.R | 10 +- R/0_linters.R | 156 ++++++++++++++---- inst/WORDLIST | 1 + man/DiseasystoreBase.Rd | 6 +- man/diseasy_linters.Rd | 57 ------- tests/testthat/test-0_R6_utils.R | 85 +++++++--- tests/testthat/test-0_documentation.R | 9 +- tests/testthat/test-0_linters.R | 34 +++- tests/testthat/test-0_non_ascii.R | 53 ------ .../{test-0_examples.R => test-0_rd_files.R} | 35 ++-- tests/testthat/test-0_return.R | 56 ------- 16 files changed, 263 insertions(+), 265 deletions(-) delete mode 100644 man/diseasy_linters.Rd delete mode 100644 tests/testthat/test-0_non_ascii.R rename tests/testthat/{test-0_examples.R => test-0_rd_files.R} (64%) delete mode 100644 tests/testthat/test-0_return.R diff --git a/.github/sync.yaml b/.github/sync.yaml index ba26da91..afaf6333 100644 --- a/.github/sync.yaml +++ b/.github/sync.yaml @@ -11,8 +11,7 @@ ssi-dk/diseasystore: - R/0_documentation.R - tests/testthat/test-0_documentation.R - - tests/testthat/test-0_return.R - - tests/testthat/test-0_examples.R + - tests/testthat/test-0_rd_files.R ssi-dk/diseasy: @@ -28,5 +27,4 @@ ssi-dk/diseasy: - R/0_documentation.R - tests/testthat/test-0_documentation.R - - tests/testthat/test-0_return.R - - tests/testthat/test-0_examples.R + - tests/testthat/test-0_rd_files.R diff --git a/.lintr b/.lintr index 13c94853..2738e5de 100644 --- a/.lintr +++ b/.lintr @@ -1,6 +1,5 @@ linters: c( - nolint_line_length_linter(120), - nolint_position_linter(120), + diseasy_code_linters(), all_linters( line_length_linter = NULL, # We use 120, nolint-aware line length linter instead cyclocomp_linter = NULL, # Not required in diseasy style guide diff --git a/DESCRIPTION b/DESCRIPTION index 55a2363c..7ec01de6 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -43,6 +43,7 @@ Imports: Suggests: curl, knitr, + pkgload, R.utils, rmarkdown, RSQLite, diff --git a/NAMESPACE b/NAMESPACE index 5d131a47..3fb93580 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -14,8 +14,6 @@ export(key_join_count) export(key_join_max) export(key_join_min) export(key_join_sum) -export(nolint_line_length_linter) -export(nolint_position_linter) export(to_diseasystore_case) importFrom(R6,R6Class) importFrom(dbplyr,window_order) diff --git a/R/0_R6_utils.R b/R/0_R6_utils.R index 2dcfd4bc..9f1041d4 100644 --- a/R/0_R6_utils.R +++ b/R/0_R6_utils.R @@ -56,7 +56,7 @@ printr <- function(..., file = nullfile(), sep = "", max_width = NULL) { stringr::str_replace(., paste0("(?<=^.{", split_width - 1, "})(\\w*) "), "\\1\n") } ) |> - stringr::str_split("\n") |> + stringr::str_split(stringr::fixed("\n")) |> purrr::reduce(c) segment_lengths <- purrr::map_dbl(split_string, ~ length(stringr::str_split_1(., " "))) @@ -84,7 +84,7 @@ printr <- function(..., file = nullfile(), sep = "", max_width = NULL) { #' # Retrieve DiseasystoreGoogleCovid19 specific option for source conn #' diseasyoption("source_conn", "DiseasystoreGoogleCovid19") #' -#' # Try to retrieve specific option for source conn for a non existant / unconfigured diseasystore +#' # Try to retrieve specific option for source conn for a non existent / un-configured diseasystore #' diseasyoption("source_conn", "DiseasystoreNonExistent") # Returns default source_conn #' @export diseasyoption <- function(option, class = "DiseasystoreBase") { @@ -93,7 +93,7 @@ diseasyoption <- function(option, class = "DiseasystoreBase") { class <- base::class(class)[1] } - base_class <- stringr::str_extract(class, r"{^([A-Z][a-z]*)}") |> # nolint: object_usage_linter + base_class <- stringr::str_extract(class, r"{^([A-Z][a-z]*)}") |> # nolint: object_usage_linter stringr::str_to_lower() list(class, NULL) |> @@ -101,7 +101,7 @@ diseasyoption <- function(option, class = "DiseasystoreBase") { purrr::map(getOption) |> purrr::map(unlist) |> purrr::keep(purrr::negate(is.null)) |> - purrr::discard(~ is.character(.) && . == "") |> + purrr::discard(~ identical(., "")) |> purrr::pluck(1) } @@ -127,7 +127,7 @@ parse_diseasyconn <- function(conn, type = "source_conn") { if (is.null(conn)) { return(conn) } else if (is.function(conn)) { - tryCatch(conn <- conn(), + tryCatch(conn <- conn(), # nolint: implicit_assignment_linter error = \(e) stop("`conn` could not be parsed!")) return(conn) } else if (type == "target_conn" && inherits(conn, "DBIConnection")) { @@ -154,9 +154,7 @@ parse_diseasyconn <- function(conn, type = "source_conn") { #' t %.% a # 1 #' #' t$c # NULL -#' \dontrun{ -#' t %.% c # ERROR a not found in t -#' } +#' try(t %.% c) # Gives error since "c" does not exist in "t" #' @export `%.%` <- function(env, field) { field_name <- as.character(substitute(field)) diff --git a/R/0_documentation.R b/R/0_documentation.R index 107a0a58..a8824c5b 100644 --- a/R/0_documentation.R +++ b/R/0_documentation.R @@ -15,7 +15,7 @@ rd_stratification <- function(type = "param") { } -rd_diseasystore <- function(type = "param") { +rd_diseasystore_label <- function(type = "param") { checkmate::assert_choice(type, c("param", "field")) paste("(`character`)\\cr", "A character string that controls which feature store to get data from.", @@ -77,7 +77,7 @@ rd_scale <- function(type = "param") { rd_source_conn <- function(type = "param") { checkmate::assert_choice(type, c("param", "field")) - paste("source_conn\\cr", + paste("(`DBIConnection` or `file path`)\\cr", "Used to specify where data is located.", ifelse(type == "field", " Read only.", ""), "Can be `DBIConnection` or file path depending on the `diseasystore`.") @@ -120,7 +120,7 @@ rd_start_date <- function(type = "param") { rd_slice_ts <- function(type = "param") { checkmate::assert_choice(type, c("param", "field")) paste("(`Date` or `character`)\\cr", - "Date or timestamp (parseable by `as.POSIXct`) to slice the database on (used if source_conn is a database).", + "Date or timestamp (parsable by `as.POSIXct`) to slice the database on (used if source_conn is a database).", ifelse(type == "field", " Read only.", "")) } @@ -135,8 +135,8 @@ rd_end_date <- function(type = "param") { rd_.data <- function(type = "param") { # nolint: object_name_linter checkmate::assert_choice(type, c("param", "field")) - paste(".data\\cr", - "The data object on which to perform the operation.", + paste("(`any`)\\cr", + "The data object to perform the operation on.", ifelse(type == "field", " Read only.", "")) } diff --git a/R/0_linters.R b/R/0_linters.R index a1007846..1d9b7b21 100644 --- a/R/0_linters.R +++ b/R/0_linters.R @@ -1,14 +1,17 @@ -#' The custom linters of `diseasy` -#' +#' @title +#' The custom linters of `diseasy` +#' @description +#' A curated list of linters to ensure adherence to the `diseasy` documentation and code standards #' @name diseasy_linters #' @examples #' diseasy_code_linters() #' @return A list of linters -#' @export +#' @noRd diseasy_code_linters <- function() { linters <- list( - non_ascii_linter(), - todo_linter() + nolint_position_linter(120), + nolint_line_length_linter(120), + non_ascii_linter() ) return(linters) @@ -16,9 +19,32 @@ diseasy_code_linters <- function() { #' @rdname diseasy_linters +#' @description +#' nolint_position_linter: Ensure `nolint:` statements occur after the character limit +#' +#' @param length maximum line length allowed. Default is 80L (Hollerith limit). +#' @returns A list of `lintr::Lint` +#' @examples +#' ## nolint_position_linter +#' # will produce lints +#' lintr::lint( +#' text = paste0(strrep("x", 15L), "# nolint: object_name_linter"), +#' linters = c(nolint_position_linter(length = 20L), lintr::object_name_linter()) +#' ) +#' +#' # okay +#' lintr::lint( +#' text = paste0(strrep("x", 20L), "# nolint: object_name_linter"), +#' linters = c(nolint_position_linter(length = 20L), lintr::object_name_linter()) +#' ) +#' +#' @seealso +#' - [lintr::linters] for a complete list of linters available in lintr. +#' - #' @importFrom rlang .data -non_ascii_linter <- function() { - general_msg <- paste("Code should not contain non-ASCII characters") +#' @noRd +nolint_position_linter <- function(length = 80L) { + general_msg <- paste("`nolint:` statements start at", length + 1, "characters.") lintr::Linter( function(source_expression) { @@ -28,28 +54,27 @@ non_ascii_linter <- function() { return(list()) } - detection_info <- source_expression$file_lines |> - stringr::str_locate_all(stringr::regex(r"{[^\x00-\x7f]}", ignore_case = TRUE)) + nolint_info <- source_expression$content |> + stringr::str_locate_all(stringr::regex(r"{# *nolint}", ignore_case = TRUE)) - detection_info <- purrr::map2( - detection_info, - seq_along(detection_info), + nolint_info <- purrr::map2( + nolint_info, + seq_along(nolint_info), ~ dplyr::mutate(as.data.frame(.x), line_number = .y) - ) - - detection_info <- detection_info |> + ) |> purrr::reduce(rbind) |> - dplyr::filter(!is.na(.data$start)) + dplyr::filter(!is.na(.data$start)) |> + dplyr::filter(.data$start <= length) purrr::pmap( - detection_info, + nolint_info, \(start, end, line_number) { lintr::Lint( filename = source_expression$filename, line_number = line_number, column_number = start, type = "style", - message = paste(general_msg, "non-ASCII character found"), + message = paste(general_msg, "This statement starts at", start, "characters"), line = source_expression$file_lines[line_number], ranges = list(c(start, end)) ) @@ -61,8 +86,28 @@ non_ascii_linter <- function() { #' @rdname diseasy_linters -todo_linter <- function() { - general_msg <- paste("`TODO` statements should not be kept in code base:") +#' @description +#' nolint_line_length_linter: Ensure lines adhere to a given character limit, ignoring `nolint` statements +#' +#' @param length maximum line length allowed. Default is 80L (Hollerith limit). +#' @examples +#' ## nolint_line_length_linter +#' # will produce lints +#' lintr::lint( +#' text = paste0(strrep("x", 25L), "# nolint: object_name_linter."), +#' linters = c(nolint_line_length_linter(length = 20L), lintr::object_name_linter()) +#' ) +#' +#' # okay +#' lintr::lint( +#' text = paste0(strrep("x", 20L), "# nolint: object_name_linter."), +#' linters = c(nolint_line_length_linter(length = 20L), lintr::object_name_linter()) +#' ) +#' +#' @importFrom rlang .data +#' @noRd +nolint_line_length_linter <- function(length = 80L) { + general_msg <- paste("Lines should not be more than", length, "characters.") lintr::Linter( function(source_expression) { @@ -72,26 +117,81 @@ todo_linter <- function() { return(list()) } - todo_info <- source_expression$file_lines |> - stringr::str_locate_all(stringr::regex(r"{(?<=\s|^)todos?:?(?=\s|$)}", ignore_case = TRUE)) + nolint_regex <- r"{# ?no(lint|cov) ?(start|end)?:?.*}" + + file_lines_nolint_excluded <- source_expression$file_lines |> + purrr::map_chr(\(s) stringr::str_remove(s, nolint_regex)) + + line_lengths <- nchar(file_lines_nolint_excluded) + long_lines <- which(line_lengths > length) + Map(function(long_line, line_length) { + lintr::Lint( + filename = source_expression$filename, + line_number = long_line, + column_number = length + 1L, type = "style", + message = paste(general_msg, "This line is", line_length, "characters."), + line = source_expression$file_lines[long_line], + ranges = list(c(1L, line_length)) + ) + }, long_lines, line_lengths[long_lines]) + } + ) +} + - todo_info <- purrr::map2( - todo_info, - seq_along(todo_info), +#' @rdname diseasy_linters +#' @description +#' non_ascii_linter: Ensure the code base only contains ASCII symbols +#' +#' @examples +#' ## non_ascii_linter +#' # will produce lints +#' lintr::lint( +#' text = "a-å", # nolint: non_ascii_linter +#' linters = non_ascii_linter() +#' ) +#' +#' # okay +#' lintr::lint( +#' text = "a-z", # nolint: non_ascii_linter +#' linters = non_ascii_linter() +#' ) +#' +#' @importFrom rlang .data +#' @noRd +non_ascii_linter <- function() { + general_msg <- paste("Code should not contain non-ASCII characters") + + lintr::Linter( + function(source_expression) { + + # Only go over complete file + if (!lintr::is_lint_level(source_expression, "file")) { + return(list()) + } + + detection_info <- source_expression$file_lines |> + stringr::str_locate_all(stringr::regex(r"{[^\x00-\x7f]}", ignore_case = TRUE)) + + detection_info <- purrr::map2( + detection_info, + seq_along(detection_info), ~ dplyr::mutate(as.data.frame(.x), line_number = .y) - ) |> + ) + + detection_info <- detection_info |> purrr::reduce(rbind) |> dplyr::filter(!is.na(.data$start)) purrr::pmap( - todo_info, + detection_info, \(start, end, line_number) { lintr::Lint( filename = source_expression$filename, line_number = line_number, column_number = start, type = "style", - message = paste(general_msg, "`TODO` statement found"), + message = paste(general_msg, "non-ASCII character found"), line = source_expression$file_lines[line_number], ranges = list(c(start, end)) ) diff --git a/inst/WORDLIST b/inst/WORDLIST index 9236b57d..f2f658f2 100644 --- a/inst/WORDLIST +++ b/inst/WORDLIST @@ -53,6 +53,7 @@ nrow OO ORCID +parsable PascalCase POSIXct pid diff --git a/man/DiseasystoreBase.Rd b/man/DiseasystoreBase.Rd index 5526a3b6..fa717a13 100644 --- a/man/DiseasystoreBase.Rd +++ b/man/DiseasystoreBase.Rd @@ -42,7 +42,7 @@ A human readable label of the feature store. Read only.} \item{\code{end_date}}{(\code{Date})\cr Study period end. Read only.} -\item{\code{slice_ts}}{(\code{Date} or \code{character})\cr Date to slice the database on (used if source_conn is a database). Read only.} +\item{\code{slice_ts}}{(\code{Date} or \code{character})\cr Date or timestamp (parsable by \code{as.POSIXct}) to slice the database on (used if source_conn is a database). Read only.} } \if{html}{\out{}} } @@ -80,7 +80,7 @@ Creates a new instance of the \code{DiseasystoreBase} \link[R6:R6Class]{R6} clas \item{\code{end_date}}{(\code{Date})\cr Study period end.} -\item{\code{slice_ts}}{(\code{Date} or \code{character})\cr Date to slice the database on (used if source_conn is a database).} +\item{\code{slice_ts}}{(\code{Date} or \code{character})\cr Date or timestamp (parsable by \code{as.POSIXct}) to slice the database on (used if source_conn is a database).} \item{\code{source_conn}}{(\code{DBIConnection} or \verb{file path})\cr Used to specify where data is located. Can be \code{DBIConnection} or file path depending on the \code{diseasystore}.} @@ -131,7 +131,7 @@ The name of a feature defined in the feature store.} \item{\code{end_date}}{(\code{Date})\cr Study period end.} -\item{\code{slice_ts}}{(\code{Date} or \code{character})\cr Date to slice the database on (used if source_conn is a database).} +\item{\code{slice_ts}}{(\code{Date} or \code{character})\cr Date or timestamp (parsable by \code{as.POSIXct}) to slice the database on (used if source_conn is a database).} } \if{html}{\out{}} } diff --git a/man/diseasy_linters.Rd b/man/diseasy_linters.Rd deleted file mode 100644 index c9e2cbec..00000000 --- a/man/diseasy_linters.Rd +++ /dev/null @@ -1,57 +0,0 @@ -% Generated by roxygen2: do not edit by hand -% Please edit documentation in R/0_linters.R -\name{diseasy_linters} -\alias{diseasy_linters} -\alias{nolint_position_linter} -\alias{nolint_line_length_linter} -\title{The custom linters of \code{diseasy}} -\usage{ -nolint_position_linter(length = 80L) - -nolint_line_length_linter(length = 80L) -} -\arguments{ -\item{length}{maximum line length allowed. Default is 80L (Hollerith limit).} -} -\value{ -A list of \code{lintr::Lint} -} -\description{ -nolint_position_linter: Check that the \verb{nolint:} statements occur after the character limit - -nolint_line_length_linter: Check that lines adhere to a given character limit, ignoring \code{nolint} statements -} -\examples{ -## nolint_position_linter -# will produce lints -lintr::lint( - text = paste0(strrep("x", 15L), "# nolint: object_name_linter"), - linters = c(nolint_position_linter(length = 20L), lintr::object_name_linter()) -) - -# okay -lintr::lint( - text = paste0(strrep("x", 20L), "# nolint: object_name_linter"), - linters = c(nolint_position_linter(length = 20L), lintr::object_name_linter()) -) - -## nolint_line_length_linter -# will produce lints -lintr::lint( - text = paste0(strrep("x", 25L), "# nolint: object_name_linter."), - linters = c(nolint_line_length_linter(length = 20L), lintr::object_name_linter()) -) - -# okay -lintr::lint( - text = paste0(strrep("x", 20L), "# nolint: object_name_linter."), - linters = c(nolint_line_length_linter(length = 20L), lintr::object_name_linter()) -) - -} -\seealso{ -\itemize{ -\item \link[lintr:linters]{lintr::linters} for a complete list of linters available in lintr. -\item \url{https://style.tidyverse.org/syntax.html#long-lines} -} -} diff --git a/tests/testthat/test-0_R6_utils.R b/tests/testthat/test-0_R6_utils.R index 0fcecc22..ee300437 100644 --- a/tests/testthat/test-0_R6_utils.R +++ b/tests/testthat/test-0_R6_utils.R @@ -1,29 +1,60 @@ -test_that("diseasyoption works", { +test_that("printr works", { + + # Check that printr works without file printing + # 1) + checkmate::expect_character(capture.output(printr("test string")), pattern = r"{test string}") + # 2) + checkmate::expect_character(capture.output(printr("test1", "test2")), pattern = r"{test1test2}") + # 3) + checkmate::expect_character(capture.output(printr("test1", "test2", sep = " ")), pattern = r"{test1 test2}") + + # Check that printr works with file printing + test_file <- tempfile() + + # 1) + checkmate::expect_character(capture.output(printr("test string", file = test_file)), + pattern = r"{test string}") + checkmate::expect_character(readLines(test_file), + pattern = r"{test string}") + file.remove(test_file) + + + # 2) + checkmate::expect_character(capture.output(printr("test1", "test2", file = test_file)), + pattern = r"{test1test2}") + checkmate::expect_character(readLines(test_file), + pattern = r"{test1test2}") + file.remove(test_file) + + + # 3) + checkmate::expect_character(capture.output(printr("test1", "test2", file = test_file, sep = " ")), + pattern = r"{test1 test2}") + checkmate::expect_character(readLines(test_file), + pattern = r"{test1 test2}") + file.remove(test_file) +}) - # Store the current options - opts <- options( - "diseasystore.target_schema" = "", - "diseasystore.DiseasystoreGoogleCovid19.target_schema" = "" - ) + +test_that("diseasyoption works", { # Check that diseasyoption works for default values - expect_equal(diseasyoption("target_schema"), NULL) + expect_null(diseasyoption("non_existent_option")) - options("diseasystore.target_schema" = "target_schema_1") - expect_equal(diseasyoption("target_schema"), "target_schema_1") + withr::local_options("diseasystore.target_schema" = target_schema_1) + expect_identical(diseasyoption("target_schema"), target_schema_1) # Check that it works for child classes ds <- DiseasystoreGoogleCovid19$new(target_conn = DBI::dbConnect(RSQLite::SQLite())) - expect_equal(diseasyoption("target_schema", "DiseasystoreGoogleCovid19"), "target_schema_1") - expect_equal(diseasyoption("target_schema", ds), "target_schema_1") + expect_identical(diseasyoption("target_schema", "DiseasystoreGoogleCovid19"), target_schema_1) + expect_identical(diseasyoption("target_schema", ds), target_schema_1) - options("diseasystore.DiseasystoreGoogleCovid19.target_schema" = "target_schema_2") - expect_equal(diseasyoption("target_schema", "DiseasystoreGoogleCovid19"), "target_schema_2") - expect_equal(diseasyoption("target_schema", ds), "target_schema_2") + withr::local_options("diseasystore.DiseasystoreGoogleCovid19.target_schema" = target_schema_2) + expect_identical(diseasyoption("target_schema", "DiseasystoreGoogleCovid19"), target_schema_2) + expect_identical(diseasyoption("target_schema", ds), target_schema_2) - # Reset options - options(opts) - rm(opts) + rm(ds) + invisible(gc()) }) @@ -67,7 +98,7 @@ test_that("parse_diseasyconn works", { null_conn <- NULL # Test inputs for source_conn - expect_no_condition(conn <- parse_diseasyconn(valid_function_conn, type = "source_conn")) + conn <- expect_no_condition(parse_diseasyconn(valid_function_conn, type = "source_conn")) checkmate::expect_class(conn, "DBIConnection") DBI::dbDisconnect(conn) @@ -76,19 +107,21 @@ test_that("parse_diseasyconn works", { class = "simpleError", regexp = "`conn` could not be parsed!") - expect_no_condition(conn <- parse_diseasyconn(valid_dbi_conn, type = "source_conn")) + conn <- expect_no_condition(parse_diseasyconn(valid_dbi_conn, type = "source_conn")) expect_identical(conn, valid_dbi_conn) + checkmate::expect_class(conn, "DBIConnection") + - expect_no_condition(conn <- parse_diseasyconn(valid_str_conn, type = "source_conn")) + conn <- expect_no_condition(parse_diseasyconn(valid_str_conn, type = "source_conn")) expect_identical(conn, valid_str_conn) - expect_no_condition(conn <- parse_diseasyconn(null_conn, type = "source_conn")) + conn <- expect_no_condition(parse_diseasyconn(null_conn, type = "source_conn")) expect_null(conn) # Test inputs for target_conn - expect_no_condition(conn <- parse_diseasyconn(valid_function_conn, type = "target_conn")) + conn <- expect_no_condition(parse_diseasyconn(valid_function_conn, type = "target_conn")) checkmate::expect_class(conn, "DBIConnection") DBI::dbDisconnect(conn) @@ -97,14 +130,18 @@ test_that("parse_diseasyconn works", { class = "simpleError", regexp = "`conn` could not be parsed!") - expect_no_condition(conn <- parse_diseasyconn(valid_dbi_conn, type = "target_conn")) + conn <- expect_no_condition(parse_diseasyconn(valid_dbi_conn, type = "target_conn")) expect_identical(conn, valid_dbi_conn) + checkmate::expect_class(conn, "DBIConnection") expect_error(parse_diseasyconn(valid_str_conn, type = "target_conn"), class = "simpleError", regexp = "`conn` could not be parsed!") - expect_no_condition(conn <- parse_diseasyconn(null_conn, type = "target_conn")) + conn <- expect_no_condition(parse_diseasyconn(null_conn, type = "target_conn")) expect_null(conn) + + # Test clean up + DBI::dbDisconnect(valid_dbi_conn) }) diff --git a/tests/testthat/test-0_documentation.R b/tests/testthat/test-0_documentation.R index cc98b77d..759aef2f 100644 --- a/tests/testthat/test-0_documentation.R +++ b/tests/testthat/test-0_documentation.R @@ -1,12 +1,13 @@ test_that("rd_templates works", { - pkg_objects <- ls(base::getNamespace("diseasy")) + + pkg_objects <- ls(base::getNamespace(testthat::testing_package())) rd_objects <- purrr::keep(pkg_objects, ~ startsWith(., "rd_")) - rd_funs <- rd_objects[purrr::map_lgl(rd_objects, ~ rlang::is_function(get(.)))] + rd_functions <- rd_objects[purrr::map_lgl(rd_objects, ~ rlang::is_function(get(.)))] for (type in c("field", "param")) { - for (rd_fun in rd_funs) { - expect_no_condition(str <- do.call(rd_fun, args = list(type = type))) + for (rd_fun in rd_functions) { + str <- expect_no_condition(do.call(rd_fun, args = list(type = type))) checkmate::expect_character(str) } } diff --git a/tests/testthat/test-0_linters.R b/tests/testthat/test-0_linters.R index 931cc61f..80c95139 100644 --- a/tests/testthat/test-0_linters.R +++ b/tests/testthat/test-0_linters.R @@ -1,8 +1,38 @@ +test_that("nolint_position_linter works", { # nolint start: nolint_position_linter + lintr::expect_lint( + paste0(strrep("x", 5), "# nolint: object_name_linter."), + list("column_number" = 6, "type" = "style"), + c(nolint_position_linter(length = 20L), lintr::object_name_linter()) + ) + + lintr::expect_lint( + paste0(strrep("x", 20), "# nolint: object_name_linter."), + NULL, + c(nolint_position_linter(length = 20L), lintr::object_name_linter()) + ) +}) # nolint end + + +test_that("nolint_line_length_linter works", { # nolint start: nolint_position_linter + lintr::expect_lint( + paste0(strrep("x", 5), "# nolint: object_name_linter."), + list("column_number" = 5, "type" = "style"), + c(nolint_line_length_linter(length = 4L), lintr::object_name_linter()) + ) + + lintr::expect_lint( + paste0(strrep("x", 5), "# nolint: object_name_linter."), + NULL, + c(nolint_line_length_linter(length = 5L), lintr::object_name_linter()) + ) +}) # nolint end + + test_that("non_ascii_linter works", { - lintr::expect_lint("æ", list("line_number" = 1, "type" = "style"), non_ascii_linter()) # nolint start: non_ascii_linter + lintr::expect_lint("æ", list("line_number" = 1, "type" = "style"), non_ascii_linter()) # nolint start: non_ascii_linter lintr::expect_lint("\nø", list("line_number" = 2, "type" = "style"), non_ascii_linter()) lintr::expect_lint("\n\nå", list("line_number" = 3, "type" = "style"), non_ascii_linter()) - lintr::expect_lint("æ", list("line_number" = 1, "type" = "style"), non_ascii_linter()) # nolint end + lintr::expect_lint("æ", list("line_number" = 1, "type" = "style"), non_ascii_linter()) # nolint end lintr::expect_lint("xxx", NULL, non_ascii_linter()) lintr::expect_lint("abc", NULL, non_ascii_linter()) diff --git a/tests/testthat/test-0_non_ascii.R b/tests/testthat/test-0_non_ascii.R deleted file mode 100644 index d7b3ff42..00000000 --- a/tests/testthat/test-0_non_ascii.R +++ /dev/null @@ -1,53 +0,0 @@ -test_that("Code contains no non-ASCII characters", { - - # Look for the source of .Rd files - help_dir <- system.file("help", package = testthat::testing_package()) - man_dir <- system.file("man", package = testthat::testing_package()) - - # Get path to R folder - r_dir <- system.file("R", package = testthat::testing_package()) - - if (checkmate::test_directory_exists(help_dir)) { - - rdx_file <- purrr::keep(dir(help_dir, full.names = TRUE), ~ stringr::str_detect(., ".rdx$")) - rd_envir <- new.env() - lazyLoad(stringr::str_remove(rdx_file, ".rdx$"), envir = rd_envir) - rd_names <- ls(rd_envir) - rd_files <- purrr::map(rd_names, ~ as.character(eval(purrr::pluck(rd_envir, .)))) - names(rd_files) <- paste0(rd_names, ".Rd") - - } else if (checkmate::test_directory_exists(man_dir)) { - - rd_paths <- purrr::keep(dir(man_dir, full.names = TRUE), ~ stringr::str_detect(., ".[Rr][Dd]$")) - rd_files <- purrr::map(rd_paths, readLines) - names(rd_files) <- purrr::map_chr(rd_paths, basename) - - } else { - - stop(".Rd files could not be located") - - } - - # Skip the "*-package.Rd" file - rd_files <- rd_files[!stringr::str_detect(names(rd_files), "-package.[Rr][Dd]$")] - - if (checkmate::test_directory_exists(r_dir)) { - - r_paths <- purrr::keep(dir(r_dir, full.names = TRUE), ~ stringr::str_detect(., ".[Rr]$")) - r_files <- purrr::map(r_paths, readLines) - names(r_paths) <- purrr::map_chr(r_paths, basename) - - } else { - - stop(".R files could not be located") - - } - - files_to_check <- c(r_files, rd_files) - - # Check the files - for (file_id in seq_along(files_to_check)) { - has_non_ascii <- purrr::some(files_to_check[[file_id]], ~ stringr::str_detect(., r"{[^\x00-\x7f]}")) - expect_false(has_non_ascii, label = paste("File:", names(files_to_check)[[file_id]])) - } -}) diff --git a/tests/testthat/test-0_examples.R b/tests/testthat/test-0_rd_files.R similarity index 64% rename from tests/testthat/test-0_examples.R rename to tests/testthat/test-0_rd_files.R index abe1d0e2..8fcd8e75 100644 --- a/tests/testthat/test-0_examples.R +++ b/tests/testthat/test-0_rd_files.R @@ -1,4 +1,4 @@ -test_that(r"{.Rd files have \examples}", { +test_field_in_documentation <- function(field) { # Load .Rd files based on environment # When using R-CMD-Check the deployment is different from when using devtools::test(). @@ -8,16 +8,11 @@ test_that(r"{.Rd files have \examples}", { # Note that `devtools::test()`, `devtools::check()` and GitHub workflows all have different # folder structures during testing, so we need to account for these differences - # Platform independent regex search to find root folder - path_regex <- stringr::str_replace(r"{(.*\/\w*(.Rcheck)?)\/.*(?=\/testthat)}", "/", .Platform$file.sep) - pkg_path <- stringr::str_extract(getwd(), path_regex, group = 1) - - # If path contains .Rcheck, we need to add the package name to the path - pkg_path <- stringr::str_replace(pkg_path, r"{(\w*)(\.Rcheck)}", paste0("\\1\\2", .Platform$file.sep, "\\1")) - # Look for the source of .Rd files - help_dir <- file.path(pkg_path, "help") - man_dir <- file.path(pkg_path, "man") + help_dir <- system.file("help", package = testthat::testing_package()) + man_dir <- system.file("man", package = testthat::testing_package()) + + testthat::expect_true(any(dir.exists(c(help_dir, man_dir)))) if (checkmate::test_directory_exists(help_dir)) { @@ -25,8 +20,7 @@ test_that(r"{.Rd files have \examples}", { rd_envir <- new.env() lazyLoad(stringr::str_remove(rdx_file, ".rdx$"), envir = rd_envir) rd_names <- ls(rd_envir) - rd_files <- purrr::map(rd_names, ~ as.character(eval(purrr::pluck(rd_envir, .)))) |> - purrr::map(~ paste(., collapse = "")) + rd_files <- purrr::map(rd_names, ~ as.character(eval(purrr::pluck(rd_envir, .)))) names(rd_files) <- paste0(rd_names, ".Rd") } else if (checkmate::test_directory_exists(man_dir)) { @@ -45,12 +39,19 @@ test_that(r"{.Rd files have \examples}", { # Skip the "*-package.Rd" file rd_files <- rd_files[!stringr::str_detect(names(rd_files), "-package.[Rr][Dd]$")] - # Skip the "data" files - rd_files <- purrr::discard(rd_files, ~ any(stringr::str_detect(., r"{\\keyword\{data\}}"))) - # Check renaming for (rd_id in seq_along(rd_files)) { - has_example <- any(stringr::str_detect(rd_files[[rd_id]], r"{\\example}")) - expect_true(has_example, label = paste("File:", names(rd_files)[[rd_id]])) + has_field <- any(stringr::str_detect(rd_files[[rd_id]], paste0(r"{\\}", field))) + testthat::expect_true(has_field, label = paste("File:", names(rd_files)[[rd_id]])) } +} + + +test_that(r"{.Rd files have \examples}", { + test_field_in_documentation("example") +}) + + +test_that(r"{.Rd files have \value}", { + test_field_in_documentation("value") }) diff --git a/tests/testthat/test-0_return.R b/tests/testthat/test-0_return.R deleted file mode 100644 index 4564dd48..00000000 --- a/tests/testthat/test-0_return.R +++ /dev/null @@ -1,56 +0,0 @@ -test_that(r"{.Rd files have \Value}", { - - # Load .Rd files based on environment - # When using R-CMD-Check the deployment is different from when using devtools::test(). - # Deployment on Github is also different from running R-CMD-Check locally - # Here we need to read directly from a .rdx database stored in the "help" folder - # If we are testing locally, we read from the "man" folder - # Note that `devtools::test()`, `devtools::check()` and GitHub workflows all have different - # folder structures during testing, so we need to account for these differences - - # Platform independent regex search to find root folder - path_regex <- stringr::str_replace(r"{(.*\/\w*(.Rcheck)?)\/.*(?=\/testthat)}", "/", .Platform$file.sep) - pkg_path <- stringr::str_extract(getwd(), path_regex, group = 1) - - # If path contains .Rcheck, we need to add the package name to the path - pkg_path <- stringr::str_replace(pkg_path, r"{(\w*)(\.Rcheck)}", paste0("\\1\\2", .Platform$file.sep, "\\1")) - - # Look for the source of .Rd files - help_dir <- file.path(pkg_path, "help") - man_dir <- file.path(pkg_path, "man") - - if (checkmate::test_directory_exists(help_dir)) { - - rdx_file <- purrr::keep(dir(help_dir, full.names = TRUE), ~ stringr::str_detect(., ".rdx$")) - rd_envir <- new.env() - lazyLoad(stringr::str_remove(rdx_file, ".rdx$"), envir = rd_envir) - rd_names <- ls(rd_envir) - rd_files <- purrr::map(rd_names, ~ as.character(eval(purrr::pluck(rd_envir, .)))) |> - purrr::map(~ paste(., collapse = "")) - names(rd_files) <- paste0(rd_names, ".Rd") - - } else if (checkmate::test_directory_exists(man_dir)) { - - rd_paths <- purrr::keep(dir(man_dir, full.names = TRUE), ~ stringr::str_detect(., ".[Rr][Dd]$")) - rd_files <- purrr::map(rd_paths, readLines) - names(rd_files) <- purrr::map_chr(rd_paths, basename) - - } else { - - stop(".Rd files could not be located") - - } - - - # Skip the "*-package.Rd" file - rd_files <- rd_files[!stringr::str_detect(names(rd_files), "-package.[Rr][Dd]$")] - - # Skip the "data" files - rd_files <- purrr::discard(rd_files, ~ any(stringr::str_detect(., r"{\\keyword\{data\}}"))) - - # Check renaming - for (rd_id in seq_along(rd_files)) { - has_value <- any(stringr::str_detect(rd_files[[rd_id]], r"{\\value}")) - expect_true(has_value, label = paste("File:", names(rd_files)[[rd_id]])) - } -}) From 12be398e106dd5237dd02b0a314d9ad387a132c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rasmus=20Skytte=20Randl=C3=B8v?= Date: Thu, 21 Dec 2023 13:59:45 +0100 Subject: [PATCH 12/14] feat(synchronise-files): add .lintr, pkgdown and WORDLIST --- .github/sync.yaml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.github/sync.yaml b/.github/sync.yaml index afaf6333..68e6fd87 100644 --- a/.github/sync.yaml +++ b/.github/sync.yaml @@ -2,6 +2,10 @@ ssi-dk/diseasystore: - .github/workflows/synchronise-files.yaml - .github/sync.yaml + - .lintr + - _pkgdown.yml + - inst/WORDLIST + - R/0_linters.R - tests/testthat/test-0_linters.R @@ -18,6 +22,10 @@ ssi-dk/diseasy: - .github/workflows/synchronise-files.yaml - .github/sync.yaml + - .lintr + - _pkgdown.yml + - inst/WORDLIST + - R/0_linters.R - tests/testthat/test-0_linters.R From 5c13e0e54ec4f9fdfe393d6b4e2c9c7933b56c29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rasmus=20Skytte=20Randl=C3=B8v?= Date: Thu, 21 Dec 2023 15:27:49 +0100 Subject: [PATCH 13/14] feat(synchronise-files): don't commit each file separately --- .github/workflows/synchronise-files.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/synchronise-files.yaml b/.github/workflows/synchronise-files.yaml index aa967978..8089e31a 100644 --- a/.github/workflows/synchronise-files.yaml +++ b/.github/workflows/synchronise-files.yaml @@ -19,3 +19,4 @@ jobs: CONFIG_PATH: .github/sync.yaml COMMIT_PREFIX: "chore: " PR_BODY: Automatically synchronise files between diseasyverse repositories + COMMIT_EACH_FILE: false From 1411f8ac8785915ac9e508a91c7ef384b4141a88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rasmus=20Skytte=20Randl=C3=B8v?= Date: Fri, 22 Dec 2023 09:02:55 +0100 Subject: [PATCH 14/14] feat(all-workflows): adjust triggering during PR --- .github/workflows/all-workflows.yaml | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/.github/workflows/all-workflows.yaml b/.github/workflows/all-workflows.yaml index ae897cac..8263f03f 100644 --- a/.github/workflows/all-workflows.yaml +++ b/.github/workflows/all-workflows.yaml @@ -1,17 +1,11 @@ -# Workflow derived from https://github.com/r-lib/actions/tree/v2/examples -# Need help debugging build failures? Start at https://github.com/r-lib/actions#where-to-find-help -on: - push: - pull_request: - types: [opened, reopened] - release: - workflow_dispatch: +on: [push, pull_request, release, workflow_dispatch] name: CI tests jobs: # We call the reusable workflow that triggers all AEF-DDF workflows run-all-AEF-DFF-workflows: + if: github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.event.pull_request.base.repo.full_name name: ⚙️ Run all AEF-DDF workflows uses: ssi-dk/AEF-DDF/.github/workflows/workflow-dispatcher.yaml@workflows with: