From 18fc8e19d6513105047e54cf1652861f5bb677b7 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Fri, 21 Oct 2022 15:52:45 +0200 Subject: [PATCH] Simplify non-consecutive errors --- R/subscript-loc.R | 39 ++++++------------------- tests/testthat/_snaps/subscript-loc.md | 40 +++++++++++++++----------- 2 files changed, 32 insertions(+), 47 deletions(-) diff --git a/R/subscript-loc.R b/R/subscript-loc.R index d3e6012b1..f8bfc1de2 100644 --- a/R/subscript-loc.R +++ b/R/subscript-loc.R @@ -489,15 +489,10 @@ stop_subscript_oob <- function(i, #' @export cnd_body.vctrs_error_subscript_oob <- function(cnd, ...) { - switch(cnd_subscript_type(cnd), - numeric = - if (cnd_subscript_oob_non_consecutive(cnd)) { - cnd_body_vctrs_error_subscript_oob_non_consecutive(cnd, ...) - } else { - cnd_body_vctrs_error_subscript_oob_location(cnd, ...) - }, - character = - cnd_body_vctrs_error_subscript_oob_name(cnd, ...), + switch( + cnd_subscript_type(cnd), + numeric = cnd_body_vctrs_error_subscript_oob_location(cnd, ...), + character = cnd_body_vctrs_error_subscript_oob_name(cnd, ...), abort("Internal error: subscript type can't be `logical` for OOB errors.") ) } @@ -532,10 +527,13 @@ cnd_body_vctrs_error_subscript_oob_location <- function(cnd, ...) { not <- "" } + allow_extension <- cnd_subscript_oob_non_consecutive(cnd) + # TODO: Switch to `format_inline()` and format bullets lazily through rlang cli::format_error(c( "x" = "{cli::qty(n_loc)} Location{?s} must be less than or equal to {n}{not}.", - "i" = "There {cli::qty(n)} {?is/are} only {elt}." + "i" = "There {cli::qty(n)} {?is/are} only {elt}.", + "i" = if (allow_extension) "Extension with consecutive locations is allowed." )) } cnd_body_vctrs_error_subscript_oob_name <- function(cnd, ...) { @@ -570,27 +568,6 @@ stop_location_oob_non_consecutive <- function(i, ) } -cnd_body_vctrs_error_subscript_oob_non_consecutive <- function(cnd, ...) { - i <- sort(cnd$i) - i <- i[i > cnd$size] - - non_consecutive <- i[c(TRUE, diff(i) != 1L)] - - arg <- append_arg("Subscript", cnd$subscript_arg) - if (length(non_consecutive) == 1) { - x_line <- glue::glue("{arg} contains non-consecutive location {non_consecutive}.") - } else { - non_consecutive <- ensure_full_stop(enumerate(non_consecutive)) - x_line <- glue::glue("{arg} contains non-consecutive locations {non_consecutive}") - } - - glue_data_bullets( - cnd, - i = "Input has size {size}.", - x = x_line - ) -} - cnd_subscript_oob_non_consecutive <- function(cnd) { out <- cnd$subscript_oob_non_consecutive %||% FALSE check_bool(out) diff --git a/tests/testthat/_snaps/subscript-loc.md b/tests/testthat/_snaps/subscript-loc.md index 36637d7d3..4606c4086 100644 --- a/tests/testthat/_snaps/subscript-loc.md +++ b/tests/testthat/_snaps/subscript-loc.md @@ -416,8 +416,9 @@ Error: ! Can't subset elements with `3`. - i Input has size 1. - x Subscript `3` contains non-consecutive location 3. + x Location must be less than or equal to 1, not 3. + i There is only 1 element. + i Extension with consecutive locations is allowed. Code (expect_error(num_as_location(c(1, 3), 1, oob = "extend"), class = "vctrs_error_subscript_oob") ) @@ -425,8 +426,9 @@ Error: ! Can't subset elements with `c(1, 3)`. - i Input has size 1. - x Subscript `c(1, 3)` contains non-consecutive location 3. + x Location must be less than or equal to 1. + i There is only 1 element. + i Extension with consecutive locations is allowed. Code (expect_error(num_as_location(c(1:5, 7), 3, oob = "extend"), class = "vctrs_error_subscript_oob") ) @@ -434,8 +436,9 @@ Error: ! Can't subset elements with `c(1:5, 7)`. - i Input has size 3. - x Subscript `c(1:5, 7)` contains non-consecutive locations 4 and 7. + x Locations must be less than or equal to 3. + i There are only 3 elements. + i Extension with consecutive locations is allowed. Code (expect_error(num_as_location(c(1:5, 7, 1), 3, oob = "extend"), class = "vctrs_error_subscript_oob") ) @@ -443,8 +446,9 @@ Error: ! Can't subset elements with `c(1:5, 7, 1)`. - i Input has size 3. - x Subscript `c(1:5, 7, 1)` contains non-consecutive locations 4 and 7. + x Locations must be less than or equal to 3. + i There are only 3 elements. + i Extension with consecutive locations is allowed. Code (expect_error(class = "vctrs_error_subscript_oob", num_as_location(c(1:5, 7, 1, 10), 3, oob = "extend"))) @@ -452,8 +456,9 @@ Error: ! Can't subset elements with `c(1:5, 7, 1, 10)`. - i Input has size 3. - x Subscript `c(1:5, 7, 1, 10)` contains non-consecutive locations 4, 7, and 10. + x Locations must be less than or equal to 3. + i There are only 3 elements. + i Extension with consecutive locations is allowed. # num_as_location() errors when inverting oob negatives unless `oob = 'remove'` (#1630) @@ -553,8 +558,9 @@ Error: ! Can't subset elements with `c(1, NA, 3)`. - i Input has size 1. - x Subscript `c(1, NA, 3)` contains non-consecutive location 3. + x Location must be less than or equal to 1. + i There is only 1 element. + i Extension with consecutive locations is allowed. # can disallow missing values @@ -778,8 +784,9 @@ Error in `my_function()`: ! Can't subset elements with `foo`. - i Input has size 2. - x Subscript `foo` contains non-consecutive location 4. + x Location must be less than or equal to 2. + i There are only 2 elements. + i Extension with consecutive locations is allowed. Code (expect_error(num_as_location(0, 1, zero = "error", arg = "foo", call = call( "my_function")), class = "vctrs_error_subscript_type")) @@ -863,8 +870,9 @@ Error: ! Can't rename columns with `foo(bar)`. - i Input has size 2. - x Subscript `foo(bar)` contains non-consecutive location 4. + x Location must be less than or equal to 2. + i There are only 2 columns. + i Extension with consecutive locations is allowed. Code (expect_error(with_tibble_cols(num_as_location(0, 1, zero = "error")), class = "vctrs_error_subscript_type") )