Ak/clean up hub_validation print output. Deprecate file_modification_check argument "warn"

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,

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.

R/check_valid_round_id_col.R

@@ -5,7 +5,7 @@
 #' @return
 #' Depending on whether validation has succeeded, one of:
 #' - `<message/check_success>` condition class object.
-#' - `<warning/check_warning>` condition class object.
+#' - `<error/check_failure>` condition class object.
 #'
 #' If `round_id_from_variable: false` and no `round_id_col` name is provided,
 #' check is skipped and a `<message/check_info>` condition class object is

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), " ",

R/validate_pr.R

@@ -13,7 +13,7 @@
 #' or deletion of a previously submitted model metadata file is detected in PR:
 #' - `"error"`: Appends a `<error/check_error>` condition class object for each
 #' applicable modified/deleted file.
-#' - `"warning"`: Appends a `<warning/check_warning>` condition class object for each
+#' - `"warning"`: Appends a `<error/check_failure>` condition class object for each
 #' applicable modified/deleted file.
 #' - `"message"`: Appends a `<message/check_info>` 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)

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.

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.

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))

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 `<error/check_error>` condition class object for each applicable modified/deleted file. Will result in validation workflow failure.
-  - **`"warning"`**: Appends a `<warning/check_warning>` condition class object for each applicable modified/deleted file. Will result in validation workflow failure.
+  - **`"failure"`**: Appends a `<error/check_failure>` condition class object for each applicable modified/deleted file. Will result in validation workflow failure.
   - **`"message"`**: Appends a `<message/check_info>` condition class object for each applicable modified/deleted file. Will not result in validation workflow failure.
   - **`"none"`**: No modification/deletion checks performed.