Skip to content

Commit

Permalink
Simplify non-consecutive errors
Browse files Browse the repository at this point in the history
  • Loading branch information
lionel- committed Oct 21, 2022
1 parent c780043 commit 18fc8e1
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 47 deletions.
39 changes: 8 additions & 31 deletions R/subscript-loc.R
Original file line number Diff line number Diff line change
Expand Up @@ -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.")
)
}
Expand Down Expand Up @@ -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, ...) {
Expand Down Expand Up @@ -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)
Expand Down
40 changes: 24 additions & 16 deletions tests/testthat/_snaps/subscript-loc.md
Original file line number Diff line number Diff line change
Expand Up @@ -416,44 +416,49 @@
<error/vctrs_error_subscript_oob>
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")
)
Output
<error/vctrs_error_subscript_oob>
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")
)
Output
<error/vctrs_error_subscript_oob>
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")
)
Output
<error/vctrs_error_subscript_oob>
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")))
Output
<error/vctrs_error_subscript_oob>
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)

Expand Down Expand Up @@ -553,8 +558,9 @@
<error/vctrs_error_subscript_oob>
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

Expand Down Expand Up @@ -778,8 +784,9 @@
<error/vctrs_error_subscript_oob>
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"))
Expand Down Expand Up @@ -863,8 +870,9 @@
<error/vctrs_error_subscript_oob>
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")
)
Expand Down

0 comments on commit 18fc8e1

Please sign in to comment.