diff --git a/NEWS.md b/NEWS.md index 68d3530a..eb703c4f 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,8 @@ # dials (development version) +* Improving error messages by switching to cli (#315). + + # dials 1.2.0 ## New parameters diff --git a/R/aaa_values.R b/R/aaa_values.R index ab2e2029..ad097256 100644 --- a/R/aaa_values.R +++ b/R/aaa_values.R @@ -312,9 +312,7 @@ value_set <- function(object, values) { if (length(values) == 0) { rlang::abort("`values` should at least one element.") } - if (!inherits(object, "param")) { - rlang::abort("`object` should be a 'param' object") - } + check_param(object) if (inherits(object, "quant_param")) { object <- diff --git a/R/constructors.R b/R/constructors.R index bb9b8ae7..58dbe0cc 100644 --- a/R/constructors.R +++ b/R/constructors.R @@ -112,10 +112,13 @@ new_quant_param <- function(type = c("double", "integer"), } if (is.null(range)) { - rlang::abort("`range` must be supplied if `values` is `NULL`.", call = call) + cli::cli_abort( + "{.arg range} must be supplied if {.arg values} is `NULL`.", + call = call + ) } if (is.null(inclusive)) { - rlang::abort( + cli::cli_abort( "`inclusive` must be supplied if `values` is `NULL`.", call = call ) @@ -129,16 +132,14 @@ new_quant_param <- function(type = c("double", "integer"), check_inclusive(inclusive) - if (!is.null(trans)) { - if (!is.trans(trans)) { - rlang::abort( - c( - "`trans` must be a 'trans' class object (or `NULL`).", - i = "See `?scales::trans_new`." - ), - call = call - ) - } + if (!is.null(trans) && !is.trans(trans)) { + cli::cli_abort( + c( + x = "{.arg trans} must be a {.cls trans} class object (or `NULL`).", + i = "See {.fn scales::trans_new}." + ), + call = call + ) } check_label(label, call = call) @@ -158,20 +159,16 @@ new_quant_param <- function(type = c("double", "integer"), if (!is.null(values)) { ok_vals <- value_validate(res, values, call = call) - if (all(ok_vals)) { - res$values <- values - } else { - msg <- paste0( - "Some values are not valid: ", - glue_collapse( - values[!ok_vals], - sep = ", ", - last = " and ", - width = min(options()$width - 30, 10) - ) + + if (!all(ok_vals)) { + offenders <- values[!ok_vals] + cli::cli_abort( + "Some values are not valid: {.val {offenders}}.", + call = call ) - rlang::abort(msg, call = call) } + + res$values <- values } res diff --git a/R/finalize.R b/R/finalize.R index 42011695..88dc198d 100644 --- a/R/finalize.R +++ b/R/finalize.R @@ -152,9 +152,7 @@ finalize.default <- function(object, x, force = TRUE, ...) { #' @export #' @rdname finalize get_p <- function(object, x, log_vals = FALSE, ...) { - if (!inherits(object, "param")) { - rlang::abort("`object` should be a 'param' object.") - } + check_param(object) rngs <- range_get(object, original = FALSE) if (!is_unknown(rngs$upper)) { @@ -188,9 +186,8 @@ get_log_p <- function(object, x, ...) { #' @export #' @rdname finalize get_n_frac <- function(object, x, log_vals = FALSE, frac = 1/3, ...) { - if (!inherits(object, "param")) { - rlang::abort("`object` should be a 'param' object.") - } + check_param(object) + rngs <- range_get(object, original = FALSE) if (!is_unknown(rngs$upper)) { return(object) diff --git a/R/misc.R b/R/misc.R index 86b17187..2f494458 100644 --- a/R/misc.R +++ b/R/misc.R @@ -142,3 +142,23 @@ check_inclusive <- function(x, ..., call = caller_env()) { invisible(x) } +check_param <- function(x, + ..., + allow_na = FALSE, + allow_null = FALSE, + arg = caller_arg(x), + call = caller_env()) { + if (!missing(x) && inherits(x, "param")) { + return(invisible(NULL)) + } + + stop_input_type( + x, + c("a single parameter object"), + ..., + allow_na = allow_na, + allow_null = allow_null, + arg = arg, + call = call + ) +} diff --git a/R/parameters.R b/R/parameters.R index cf6c047a..befc8ac4 100644 --- a/R/parameters.R +++ b/R/parameters.R @@ -281,12 +281,12 @@ update.parameters <- function(object, ...) { not_null <- !purrr::map_lgl(args, ~ all(is.na(.x))) bad_input <- not_param & not_null if (any(bad_input)) { - msg <- paste0("'", nms[bad_input], "'", collapse = ", ") - msg <- paste( - "At least one parameter is not a dials parameter object", - "or NA:", msg + offenders <- nms[bad_input] + + cli::cli_abort( + "At least one parameter is not a dials parameter object or NA: \\ + {offenders}." ) - rlang::abort(msg) } for (p in nms) { diff --git a/R/validation.R b/R/validation.R index aa130cc0..7ea21208 100644 --- a/R/validation.R +++ b/R/validation.R @@ -2,30 +2,26 @@ validate_params <- function(..., call = caller_env()) { param_quos <- quos(...) param_expr <- purrr::map_chr(param_quos, rlang::quo_text) if (length(param_quos) == 0) { - rlang::abort("At least one parameter object is required.", call = call) + cli::cli_abort("At least one parameter object is required.", call = call) } params <- map(param_quos, eval_tidy) is_param <- map_lgl(params, function(x) inherits(x, "param")) if (!all(is_param)) { - rlang::abort( - paste0( - "These arguments must have class 'param': ", - paste0("`", param_expr[!is_param], "`", collapse = ",") - ), + offenders <- param_expr[!is_param] + cli::cli_abort( + "Th{?is/ese} argument{?s} must have class {.cls param}: \\ + {.arg {offenders}}.", call = call ) } bad_param <- has_unknowns(params) if (any(bad_param)) { bad_param <- names(bad_param)[bad_param] - rlang::abort( + cli::cli_abort( c( - paste0( - "These arguments contain unknowns: ", - paste0("`", bad_param, "`", collapse = ","), - "." - ), - i = "See the `finalize()` function." + x = "Th{?is/ese} argument{?s} contain{?s/} unknowns: \\ + {.arg {bad_param}}.", + i = "See the {.fn dials::finalize} function." ), call = call ) diff --git a/tests/testthat/_snaps/aaa_unknown.md b/tests/testthat/_snaps/aaa_unknown.md index 14256216..61c3bb64 100644 --- a/tests/testthat/_snaps/aaa_unknown.md +++ b/tests/testthat/_snaps/aaa_unknown.md @@ -4,8 +4,8 @@ grid_regular(p1) Condition Error in `grid_regular()`: - ! These arguments contain unknowns: `q`. - i See the `finalize()` function. + x This argument contains unknowns: `q`. + i See the `dials::finalize()` function. --- @@ -13,8 +13,17 @@ grid_regular(p2) Condition Error in `grid_regular()`: - ! These arguments contain unknowns: `mtry`. - i See the `finalize()` function. + x This argument contains unknowns: `mtry`. + i See the `dials::finalize()` function. + +--- + + Code + grid_regular(p3) + Condition + Error in `grid_regular()`: + x These arguments contain unknowns: `mtry` and `q`. + i See the `dials::finalize()` function. --- @@ -22,8 +31,8 @@ grid_random(p1) Condition Error in `grid_random()`: - ! These arguments contain unknowns: `q`. - i See the `finalize()` function. + x This argument contains unknowns: `q`. + i See the `dials::finalize()` function. --- @@ -31,8 +40,8 @@ grid_random(p2) Condition Error in `grid_random()`: - ! These arguments contain unknowns: `mtry`. - i See the `finalize()` function. + x This argument contains unknowns: `mtry`. + i See the `dials::finalize()` function. --- @@ -40,8 +49,8 @@ grid_latin_hypercube(p1) Condition Error in `grid_latin_hypercube()`: - ! These arguments contain unknowns: `q`. - i See the `finalize()` function. + x This argument contains unknowns: `q`. + i See the `dials::finalize()` function. --- @@ -49,8 +58,8 @@ grid_latin_hypercube(p2) Condition Error in `grid_latin_hypercube()`: - ! These arguments contain unknowns: `mtry`. - i See the `finalize()` function. + x This argument contains unknowns: `mtry`. + i See the `dials::finalize()` function. --- @@ -58,8 +67,8 @@ grid_max_entropy(p1) Condition Error in `grid_max_entropy()`: - ! These arguments contain unknowns: `q`. - i See the `finalize()` function. + x This argument contains unknowns: `q`. + i See the `dials::finalize()` function. --- @@ -67,8 +76,8 @@ grid_max_entropy(p2) Condition Error in `grid_max_entropy()`: - ! These arguments contain unknowns: `mtry`. - i See the `finalize()` function. + x This argument contains unknowns: `mtry`. + i See the `dials::finalize()` function. --- @@ -76,8 +85,8 @@ grid_regular(min_n(), q = mtry()) Condition Error in `grid_regular()`: - ! These arguments contain unknowns: `q`. - i See the `finalize()` function. + x This argument contains unknowns: `q`. + i See the `dials::finalize()` function. --- @@ -85,8 +94,8 @@ grid_regular(mtry()) Condition Error in `grid_regular()`: - ! These arguments contain unknowns: `mtry`. - i See the `finalize()` function. + x This argument contains unknowns: `mtry`. + i See the `dials::finalize()` function. --- @@ -94,8 +103,8 @@ grid_random(min_n(), q = mtry()) Condition Error in `grid_random()`: - ! These arguments contain unknowns: `q`. - i See the `finalize()` function. + x This argument contains unknowns: `q`. + i See the `dials::finalize()` function. --- @@ -103,8 +112,8 @@ grid_random(mtry()) Condition Error in `grid_random()`: - ! These arguments contain unknowns: `mtry`. - i See the `finalize()` function. + x This argument contains unknowns: `mtry`. + i See the `dials::finalize()` function. --- @@ -112,8 +121,8 @@ grid_regular(min_n(), q = mtry()) Condition Error in `grid_regular()`: - ! These arguments contain unknowns: `q`. - i See the `finalize()` function. + x This argument contains unknowns: `q`. + i See the `dials::finalize()` function. --- @@ -121,8 +130,8 @@ grid_latin_hypercube(mtry()) Condition Error in `grid_latin_hypercube()`: - ! These arguments contain unknowns: `mtry`. - i See the `finalize()` function. + x This argument contains unknowns: `mtry`. + i See the `dials::finalize()` function. --- @@ -130,8 +139,8 @@ grid_max_entropy(min_n(), q = mtry()) Condition Error in `grid_max_entropy()`: - ! These arguments contain unknowns: `q`. - i See the `finalize()` function. + x This argument contains unknowns: `q`. + i See the `dials::finalize()` function. --- @@ -139,6 +148,6 @@ grid_max_entropy(mtry()) Condition Error in `grid_max_entropy()`: - ! These arguments contain unknowns: `mtry`. - i See the `finalize()` function. + x This argument contains unknowns: `mtry`. + i See the `dials::finalize()` function. diff --git a/tests/testthat/_snaps/constructors.md b/tests/testthat/_snaps/constructors.md index 9916b507..51480afa 100644 --- a/tests/testthat/_snaps/constructors.md +++ b/tests/testthat/_snaps/constructors.md @@ -84,8 +84,8 @@ new_quant_param("integer", range = 1:2, inclusive = c(TRUE, TRUE), trans = log) Condition Error: - ! `trans` must be a 'trans' class object (or `NULL`). - i See `?scales::trans_new`. + x `trans` must be a class object (or `NULL`). + i See `scales::trans_new()`. --- @@ -93,7 +93,7 @@ new_quant_param("integer", range = 1:2, inclusive = c(TRUE, TRUE), values = 1:4) Condition Error: - ! Some values are not valid: 3 and 4 + ! Some values are not valid: 3 and 4. --- @@ -227,7 +227,7 @@ label = c(foo = "Foo")) Condition Error: - ! Some values are not valid: 10 + ! Some values are not valid: 10. --- @@ -236,7 +236,7 @@ FALSE), label = c(foo = "Foo")) Condition Error: - ! Some values are not valid: 10 + ! Some values are not valid: 10. --- diff --git a/tests/testthat/_snaps/finalize.md b/tests/testthat/_snaps/finalize.md index b33d06e7..11c6b785 100644 --- a/tests/testthat/_snaps/finalize.md +++ b/tests/testthat/_snaps/finalize.md @@ -4,7 +4,7 @@ get_p(1:10) Condition Error in `get_p()`: - ! `object` should be a 'param' object. + ! `object` must be a single parameter object, not an integer vector. --- @@ -12,7 +12,7 @@ get_p(1:10, 1:10) Condition Error in `get_p()`: - ! `object` should be a 'param' object. + ! `object` must be a single parameter object, not an integer vector. --- @@ -28,7 +28,7 @@ get_n(1:10) Condition Error in `get_n_frac()`: - ! `object` should be a 'param' object. + ! `object` must be a single parameter object, not an integer vector. --- @@ -36,7 +36,7 @@ get_n(1:10, 1:10) Condition Error in `get_n_frac()`: - ! `object` should be a 'param' object. + ! `object` must be a single parameter object, not an integer vector. --- diff --git a/tests/testthat/_snaps/parameters.md b/tests/testthat/_snaps/parameters.md index 762ac505..d1979eb9 100644 --- a/tests/testthat/_snaps/parameters.md +++ b/tests/testthat/_snaps/parameters.md @@ -64,7 +64,7 @@ update(p_1, penalty = 1:2) Condition Error in `update()`: - ! At least one parameter is not a dials parameter object or NA: 'penalty' + ! At least one parameter is not a dials parameter object or NA: penalty. --- diff --git a/tests/testthat/_snaps/space_filling.md b/tests/testthat/_snaps/space_filling.md index f9a7d56d..087ac4f3 100644 --- a/tests/testthat/_snaps/space_filling.md +++ b/tests/testthat/_snaps/space_filling.md @@ -4,8 +4,8 @@ grid_max_entropy(mtry(), size = 11, original = FALSE) Condition Error in `grid_max_entropy()`: - ! These arguments contain unknowns: `mtry`. - i See the `finalize()` function. + x This argument contains unknowns: `mtry`. + i See the `dials::finalize()` function. # latin square designs @@ -13,6 +13,6 @@ grid_latin_hypercube(mtry(), size = 11, original = FALSE) Condition Error in `grid_latin_hypercube()`: - ! These arguments contain unknowns: `mtry`. - i See the `finalize()` function. + x This argument contains unknowns: `mtry`. + i See the `dials::finalize()` function. diff --git a/tests/testthat/_snaps/validation.md b/tests/testthat/_snaps/validation.md index 8a33c8cf..7fe13468 100644 --- a/tests/testthat/_snaps/validation.md +++ b/tests/testthat/_snaps/validation.md @@ -12,7 +12,7 @@ validate_params(1:2) Condition Error: - ! These arguments must have class 'param': `1:2` + ! This argument must have class : `1:2`. --- @@ -20,8 +20,8 @@ validate_params(mtry()) Condition Error: - ! These arguments contain unknowns: ``. - i See the `finalize()` function. + x This argument contains unknowns: ``. + i See the `dials::finalize()` function. --- @@ -30,6 +30,6 @@ validate_params(unfinalized_param) Condition Error: - ! These arguments contain unknowns: ``. - i See the `finalize()` function. + x This argument contains unknowns: ``. + i See the `dials::finalize()` function. diff --git a/tests/testthat/test-aaa_unknown.R b/tests/testthat/test-aaa_unknown.R index 4064afbe..ab4c42ea 100644 --- a/tests/testthat/test-aaa_unknown.R +++ b/tests/testthat/test-aaa_unknown.R @@ -28,9 +28,11 @@ test_that("has_unknown", { test_that("unknowns in grid functions", { p1 <- parameters(q = mtry(), min_n()) p2 <- parameters(mtry()) + p3 <- parameters(mtry(), q = mtry()) expect_snapshot(error = TRUE, grid_regular(p1)) expect_snapshot(error = TRUE, grid_regular(p2)) + expect_snapshot(error = TRUE, grid_regular(p3)) expect_snapshot(error = TRUE, grid_random(p1)) expect_snapshot(error = TRUE, grid_random(p2)) expect_snapshot(error = TRUE, grid_latin_hypercube(p1))