From 7ed8a71c06b4c4802165ba9b04a614a1f7fd7b82 Mon Sep 17 00:00:00 2001 From: Anna Krystalli Date: Fri, 6 Sep 2024 12:35:44 +0100 Subject: [PATCH 1/3] Change colour form br_cyan to magenta. More visible on light backgrounds --- R/hub_validations_methods.R | 2 +- tests/testthat/_snaps/validate_model_data.md | 4 ++-- tests/testthat/_snaps/validate_model_file.md | 12 ++++++------ 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/R/hub_validations_methods.R b/R/hub_validations_methods.R index a8fb7681..b24fb400 100644 --- a/R/hub_validations_methods.R +++ b/R/hub_validations_methods.R @@ -130,7 +130,7 @@ hub_validation_theme <- list( ), "h2" = list( fmt = function(x) { - cli::col_br_cyan( + cli::col_magenta( paste0( cli::symbol$line, cli::symbol$line, " ", cli::style_underline(x), " ", diff --git a/tests/testthat/_snaps/validate_model_data.md b/tests/testthat/_snaps/validate_model_data.md index fb695db4..ebf1f75e 100644 --- a/tests/testthat/_snaps/validate_model_data.md +++ b/tests/testthat/_snaps/validate_model_data.md @@ -271,7 +271,7 @@ validate_model_data(hub_path, file_path) Message - -- 2022-10-08-team1-goodmodel.csv ---- + -- 2022-10-08-team1-goodmodel.csv ---- v [file_read]: File could be read successfully. v [valid_round_id_col]: `round_id_col` name is valid. @@ -313,7 +313,7 @@ validate_model_data(hub_path, file_path) Message - ── 2022-10-08-team1-goodmodel.csv ──── + ── 2022-10-08-team1-goodmodel.csv ──── ✔ [file_read]: File could be read successfully. ✔ [valid_round_id_col]: `round_id_col` name is valid. diff --git a/tests/testthat/_snaps/validate_model_file.md b/tests/testthat/_snaps/validate_model_file.md index a0bab3cc..b39723fc 100644 --- a/tests/testthat/_snaps/validate_model_file.md +++ b/tests/testthat/_snaps/validate_model_file.md @@ -103,7 +103,7 @@ validate_model_file(hub_path, file_path = "team1-goodmodel/2022-10-08-team1-goodmodel.csv") Message - -- 2022-10-08-team1-goodmodel.csv ---- + -- 2022-10-08-team1-goodmodel.csv ---- v [file_exists]: File exists at path model-output/team1-goodmodel/2022-10-08-team1-goodmodel.csv. v [file_name]: File name "2022-10-08-team1-goodmodel.csv" is valid. @@ -118,7 +118,7 @@ validate_model_file(hub_path, file_path = "team1-goodmodel/2022-10-15-team1-goodmodel.csv") Message - -- 2022-10-15-team1-goodmodel.csv ---- + -- 2022-10-15-team1-goodmodel.csv ---- (x) [file_exists]: File does not exist at path model-output/team1-goodmodel/2022-10-15-team1-goodmodel.csv. @@ -128,7 +128,7 @@ validate_model_file(hub_path, file_path = "team1-goodmodel/2022-10-15-hub-baseline.csv") Message - -- 2022-10-15-hub-baseline.csv ---- + -- 2022-10-15-hub-baseline.csv ---- v [file_exists]: File exists at path model-output/team1-goodmodel/2022-10-15-hub-baseline.csv. v [file_name]: File name "2022-10-15-hub-baseline.csv" is valid. @@ -183,7 +183,7 @@ validate_model_file(hub_path, file_path = "team1-goodmodel/2022-10-08-team1-goodmodel.csv") Message - ── 2022-10-08-team1-goodmodel.csv ──── + ── 2022-10-08-team1-goodmodel.csv ──── ✔ [file_exists]: File exists at path model-output/team1-goodmodel/2022-10-08-team1-goodmodel.csv. ✔ [file_name]: File name "2022-10-08-team1-goodmodel.csv" is valid. @@ -198,7 +198,7 @@ validate_model_file(hub_path, file_path = "team1-goodmodel/2022-10-15-team1-goodmodel.csv") Message - ── 2022-10-15-team1-goodmodel.csv ──── + ── 2022-10-15-team1-goodmodel.csv ──── ⓧ [file_exists]: File does not exist at path model-output/team1-goodmodel/2022-10-15-team1-goodmodel.csv. @@ -208,7 +208,7 @@ validate_model_file(hub_path, file_path = "team1-goodmodel/2022-10-15-hub-baseline.csv") Message - ── 2022-10-15-hub-baseline.csv ──── + ── 2022-10-15-hub-baseline.csv ──── ✔ [file_exists]: File exists at path model-output/team1-goodmodel/2022-10-15-hub-baseline.csv. ✔ [file_name]: File name "2022-10-15-hub-baseline.csv" is valid. From 07b0642e4de80c26ff85f6f56693d2afec384bb4 Mon Sep 17 00:00:00 2001 From: Anna Krystalli Date: Fri, 6 Sep 2024 12:44:48 +0100 Subject: [PATCH 2/3] deprecate 'warning' file_modification_check option, replace with failure. Correct out class documentation --- DESCRIPTION | 3 ++- R/check_valid_round_id_col.R | 2 +- R/validate_pr.R | 14 +++++++++++--- man/check_valid_round_id_col.Rd | 2 +- man/validate_pr.Rd | 4 ++-- tests/testthat/test-validate_pr.R | 15 ++++++++++++++- vignettes/articles/validate-pr.Rmd | 2 +- 7 files changed, 32 insertions(+), 10 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 10af9589..d5e329d8 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: hubValidations Title: Testing framework for hubverse hub validations -Version: 0.6.0 +Version: 0.6.1 Authors@R: c( person( given = "Anna", @@ -42,6 +42,7 @@ Imports: hubUtils (>= 0.1.2), jsonlite, jsonvalidate, + lifecycle, lubridate, magrittr, purrr, diff --git a/R/check_valid_round_id_col.R b/R/check_valid_round_id_col.R index 412b4d17..a5af84e4 100644 --- a/R/check_valid_round_id_col.R +++ b/R/check_valid_round_id_col.R @@ -5,7 +5,7 @@ #' @return #' Depending on whether validation has succeeded, one of: #' - `` condition class object. -#' - `` condition class object. +#' - `` condition class object. #' #' If `round_id_from_variable: false` and no `round_id_col` name is provided, #' check is skipped and a `` condition class object is diff --git a/R/validate_pr.R b/R/validate_pr.R index 0c9f1915..0a999007 100644 --- a/R/validate_pr.R +++ b/R/validate_pr.R @@ -13,7 +13,7 @@ #' or deletion of a previously submitted model metadata file is detected in PR: #' - `"error"`: Appends a `` condition class object for each #' applicable modified/deleted file. -#' - `"warning"`: Appends a `` condition class object for each +#' - `"warning"`: Appends a `` condition class object for each #' applicable modified/deleted file. #' - `"message"`: Appends a `` condition class object for each #' applicable modified/deleted file. @@ -100,7 +100,7 @@ validate_pr <- function(hub_path = ".", gh_repo, pr_number, ), validations_cfg_path = NULL, skip_submit_window_check = FALSE, file_modification_check = c( - "error", "warn", + "error", "failure", "warn", "message", "none" ), allow_submit_window_mods = TRUE, @@ -110,6 +110,14 @@ validate_pr <- function(hub_path = ".", gh_repo, pr_number, ), derived_task_ids = NULL) { file_modification_check <- rlang::arg_match(file_modification_check) + if (file_modification_check == "warn") { + lifecycle::deprecate_warn( + "0.6.1", + "validate_pr(file_modification_check = 'will not accept option `warn` in + future versions. Use `failure` instead')" + ) + file_modification_check <- "failure" + } output_type_id_datatype <- rlang::arg_match(output_type_id_datatype) model_output_dir <- get_hub_model_output_dir(hub_path) # nolint: object_name_linter model_metadata_dir <- "model-metadata" # nolint: object_name_linter @@ -251,7 +259,7 @@ check_pr_modf_del_files <- function(pr_df, file_type = c( "model_output", # nolint "model_metadata" ), - alert = c("message", "warn", "error"), + alert = c("message", "failure", "error"), allow_submit_window_mods = TRUE) { file_type <- rlang::arg_match(file_type) alert <- rlang::arg_match(alert) diff --git a/man/check_valid_round_id_col.Rd b/man/check_valid_round_id_col.Rd index 324e1475..7510ef75 100644 --- a/man/check_valid_round_id_col.Rd +++ b/man/check_valid_round_id_col.Rd @@ -31,7 +31,7 @@ config file.} Depending on whether validation has succeeded, one of: \itemize{ \item \verb{} condition class object. -\item \verb{} condition class object. +\item \verb{} condition class object. } If \code{round_id_from_variable: false} and no \code{round_id_col} name is provided, diff --git a/man/validate_pr.Rd b/man/validate_pr.Rd index 3e6db5db..306fda4d 100644 --- a/man/validate_pr.Rd +++ b/man/validate_pr.Rd @@ -13,7 +13,7 @@ validate_pr( "logical", "Date"), validations_cfg_path = NULL, skip_submit_window_check = FALSE, - file_modification_check = c("error", "warn", "message", "none"), + file_modification_check = c("error", "failure", "warn", "message", "none"), allow_submit_window_mods = TRUE, submit_window_ref_date_from = c("file", "file_path"), derived_task_ids = NULL @@ -64,7 +64,7 @@ or deletion of a previously submitted model metadata file is detected in PR: \itemize{ \item \code{"error"}: Appends a \verb{} condition class object for each applicable modified/deleted file. -\item \code{"warning"}: Appends a \verb{} condition class object for each +\item \code{"warning"}: Appends a \verb{} condition class object for each applicable modified/deleted file. \item \code{"message"}: Appends a \verb{} condition class object for each applicable modified/deleted file. diff --git a/tests/testthat/test-validate_pr.R b/tests/testthat/test-validate_pr.R index 8316dd1b..2db121c6 100644 --- a/tests/testthat/test-validate_pr.R +++ b/tests/testthat/test-validate_pr.R @@ -77,13 +77,26 @@ test_that("validate_pr flags modifications and deletions in PR", { suppressMessages(check_for_errors(mod_checks_error)) ) + # capture file_modification_check deprecation warning + expect_warning( + suppressMessages( + validate_pr( + hub_path = temp_hub, + gh_repo = "hubverse-org/ci-testhub-simple", + pr_number = 6, + skip_submit_window_check = TRUE, + file_modification_check = "warn" + ) + ) + ) + mod_checks_warn <- suppressMessages( validate_pr( hub_path = temp_hub, gh_repo = "hubverse-org/ci-testhub-simple", pr_number = 6, skip_submit_window_check = TRUE, - file_modification_check = "warn" + file_modification_check = "failure" ) ) expect_snapshot(str(mod_checks_warn)) diff --git a/vignettes/articles/validate-pr.Rmd b/vignettes/articles/validate-pr.Rmd index 608b42be..896ac158 100644 --- a/vignettes/articles/validate-pr.Rmd +++ b/vignettes/articles/validate-pr.Rmd @@ -124,7 +124,7 @@ These settings can be modified if required though the use of arguments `file_mod - **`file_modification_check`** controls whether modification/deletion checks are performed, what is returned if modifications/deletions are detected and accepts one of the following values: - **`"error"`**: Appends a `` condition class object for each applicable modified/deleted file. Will result in validation workflow failure. - - **`"warning"`**: Appends a `` condition class object for each applicable modified/deleted file. Will result in validation workflow failure. + - **`"failure"`**: Appends a `` condition class object for each applicable modified/deleted file. Will result in validation workflow failure. - **`"message"`**: Appends a `` condition class object for each applicable modified/deleted file. Will not result in validation workflow failure. - **`"none"`**: No modification/deletion checks performed. From 50282bcab92e1dfc6c97b25da9af50549b9f4e56 Mon Sep 17 00:00:00 2001 From: Anna Krystalli Date: Fri, 6 Sep 2024 12:50:00 +0100 Subject: [PATCH 3/3] Bumpe version --- NEWS.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/NEWS.md b/NEWS.md index f0ae3a7e..f96fef1b 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,8 @@ +# hubValidations 0.6.1 + +* Changed file name header colour in `hub_validations` object `print()` method to make more visible on lighter backgrounds. +* Soft deprecated `file_modification_check` argument `"warn"` option and replaced it with `"failure"` in `validate_pr()` function. + # hubValidations 0.6.0 * To make clearer that all checks resulting in `check_failure` are required to pass for files to be considered valid, `check_failure` class objects are elevated to errors (#111). Also, to make it easier for users to identify errors from visually scanning the printed output, the following custom bullets have been assigned.