Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adapt errors to next rlang #6052

Merged
merged 139 commits into from
Nov 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
139 commits
Select commit Hold shift + click to select a range
c86e953
Extract `mutate_bullets()`
lionel- Aug 31, 2021
ebdbdfe
POC: Use chained errors to provide context
lionel- Aug 31, 2021
469ca45
call =
romainfrancois Oct 14, 2021
c1bf2b6
add_computed_columns() and distinct() using call=
romainfrancois Oct 14, 2021
00bed29
simplify group_by() error from add_computed_columns() as more context…
romainfrancois Oct 14, 2021
4ef7c9c
incorporate the parent error (perhaps skipping one layer when the bul…
romainfrancois Oct 14, 2021
73d90ad
`dplyr:::internal_error` are not suitable for parent=
romainfrancois Oct 14, 2021
b813b7d
summarise_bullets(
romainfrancois Oct 14, 2021
0d57c52
Force:
romainfrancois Oct 14, 2021
df60a96
Pretend call = vec_cast_common
romainfrancois Oct 14, 2021
a15f3f7
alter the call of errors thrown by vctrs so that they don't mention s…
romainfrancois Oct 14, 2021
2e691dd
document()
romainfrancois Oct 14, 2021
1edcc62
remove redundant summarise() bullet
romainfrancois Oct 14, 2021
6a88e0b
using cnd_signal()
romainfrancois Oct 14, 2021
089c06d
Remove redundant filter() bullet
romainfrancois Oct 14, 2021
ec8518f
reword filter() error
romainfrancois Oct 14, 2021
7e26b09
factor out filter_expand()
romainfrancois Oct 14, 2021
1363bc2
factor out filter_eval()
romainfrancois Oct 14, 2021
609109e
since we use call2() in other places
romainfrancois Oct 14, 2021
8238038
document()
romainfrancois Oct 14, 2021
6b744a8
Apply suggestions from code review
romainfrancois Oct 15, 2021
1c2f176
use more explicit name for temporary fix_vctrs_call()
romainfrancois Oct 15, 2021
beffc8d
"We detected a named input"
romainfrancois Oct 15, 2021
c0bb336
using call("<...>") instead of call2("<...>")
romainfrancois Oct 15, 2021
09ee6a7
as_label(<quosure>)
romainfrancois Oct 15, 2021
fb9ac71
Apply suggestions from code review
romainfrancois Oct 15, 2021
039f983
fix_vctrs_call() gains a class= to add to the list of error classes
romainfrancois Oct 15, 2021
8817927
slice() aborts with call=
romainfrancois Oct 15, 2021
e6cd326
cnd_header() instead of conditionMessage()
romainfrancois Oct 15, 2021
931164a
Another use of `fix_vctrs_call()`
romainfrancois Oct 15, 2021
cca0eaa
fix_vctrs_call() -> fix_error() as not strictly about `vctrs::`
romainfrancois Oct 15, 2021
dd4c919
Rebrand slice() errors in slice_*() siblings
romainfrancois Oct 15, 2021
beb4f45
limit the reach of the withCallingHandlers() in slice().
romainfrancois Oct 15, 2021
8bcdd2c
fix_call(), wrap_error() from @lionel- with minor change
romainfrancois Oct 15, 2021
a29294d
try to improve slice() errors but simplify them when called from samp…
romainfrancois Oct 15, 2021
0e762a5
passing glubort(.envir) to abort() helps error ancestry printing.
romainfrancois Oct 14, 2021
e887851
adapt to .abort = identity
romainfrancois Oct 14, 2021
bf43f78
merge
romainfrancois Oct 18, 2021
0520379
check_pkg() calling abort() directly
romainfrancois Oct 18, 2021
a36e829
auto_copy() using `abort()` directly.
romainfrancois Oct 18, 2021
f1ddf15
abort(glue())
romainfrancois Oct 18, 2021
51bdae0
- bad(), use abort() explicitely instead
romainfrancois Oct 18, 2021
0bd3030
use abort() directly
romainfrancois Oct 18, 2021
60483ef
call("case_when")
romainfrancois Oct 18, 2021
804cb83
unused functions
romainfrancois Oct 18, 2021
d13b9fe
check_type()/check_class() using abort(call=)
romainfrancois Oct 18, 2021
d191059
case_when(), na_if(), if_else() using call=
romainfrancois Oct 18, 2021
c243c45
glubort() losing .abort argument
romainfrancois Oct 18, 2021
0d1d234
unused function
romainfrancois Oct 18, 2021
95f706f
use abort() directly
romainfrancois Oct 18, 2021
2f055f3
glue() later the dplyr:::filter_incompatible_size error
romainfrancois Oct 19, 2021
df741f3
glue() later errors of class dplyr:::filter_incompatible_type
romainfrancois Oct 19, 2021
80128aa
simpler abort_glue(), i.e. not glue()ing at all
romainfrancois Oct 19, 2021
c772c20
abort_glue() -> dplyr_internal_error() simplified
romainfrancois Oct 19, 2021
0eca863
local_call_step() deos not need .fn=
romainfrancois Oct 19, 2021
bd30b60
simplified local_call_step(): losing the .dot_data argument
romainfrancois Oct 19, 2021
585a7c3
redundant information in warning
romainfrancois Oct 19, 2021
0c65f0b
only pass the condition to the *_bullets() functions, the other thing…
romainfrancois Oct 19, 2021
7fba4c8
+ skip_internal_condition()
romainfrancois Oct 19, 2021
26d4c8f
started refactoring some of summarise()
romainfrancois Oct 19, 2021
5f6d48d
computing -> recycling
romainfrancois Oct 19, 2021
0dfff00
error_context does not need to inherit from environment()
romainfrancois Oct 20, 2021
592b8da
unused
romainfrancois Oct 20, 2021
60cdd84
internal stop_mutate_recycle_incompatible_size() gains expected size …
romainfrancois Oct 20, 2021
1472d77
use cnd_bullet_header() in filter_expand()
romainfrancois Oct 20, 2021
660c77f
auto_copy() using call = auto_copy()
romainfrancois Oct 20, 2021
7f35d94
context_peek() using abort(call=)
romainfrancois Oct 20, 2021
745b97f
filter.ts() using about(call = filter)
romainfrancois Oct 20, 2021
3e0bed9
check_compatible() reporting about the caller
romainfrancois Oct 20, 2021
e9113a1
call = do
romainfrancois Oct 20, 2021
c43b1a9
abort(call = recode)
romainfrancois Oct 20, 2021
1f92228
Error in desc
romainfrancois Oct 20, 2021
18b5f57
call = recode
romainfrancois Oct 20, 2021
83f7410
lead() also fixing vec_cast_common() call.
romainfrancois Oct 20, 2021
0b0c1ab
call = ungroup
romainfrancois Oct 20, 2021
af85c18
More curating the errors
romainfrancois Oct 20, 2021
32cafe2
- redundant distinct()
romainfrancois Oct 21, 2021
c561798
fix the call for rows_*()
romainfrancois Oct 21, 2021
32cb7d4
group_by_prepare(), using call = group_by
romainfrancois Oct 21, 2021
6dcfc93
change mechanism used in slice() to report differently errors from sl…
romainfrancois Oct 21, 2021
d61a7f5
set call= in across_setup() aborts
romainfrancois Oct 21, 2021
ff4bef7
call = across + parent when expanding or evaluating across() in summa…
romainfrancois Oct 21, 2021
a0b21cd
abort(bullets)
romainfrancois Oct 21, 2021
12ecf36
Apply suggestions from code review
romainfrancois Oct 22, 2021
13acbb0
resnap after review
romainfrancois Oct 22, 2021
3c04456
wrapped_error class is not used
romainfrancois Oct 22, 2021
45facbd
Update R/recode.R
romainfrancois Oct 22, 2021
e89912f
use rlang::arg_require()
romainfrancois Oct 22, 2021
c41f36d
across() taking ownership of eval_select() and vec_as_names() errors.
romainfrancois Oct 22, 2021
28b346c
+ column
romainfrancois Oct 22, 2021
22c9133
bind_rows() and bind_cols() owning vec_rbind()/vec_cbind() errors
romainfrancois Oct 22, 2021
ebdf720
coalesce() owning errors
romainfrancois Oct 22, 2021
5aa77f5
check_dots_empty(call = )
romainfrancois Oct 22, 2021
e3b24c0
don't show parent of wrapped error
romainfrancois Oct 22, 2021
e709bce
fix_call(call = ) is explicit now, as these error are always owned by…
romainfrancois Oct 22, 2021
d887cad
simplify
romainfrancois Oct 22, 2021
0d2eab5
Remove the unused error class "dplyr_error"
romainfrancois Oct 22, 2021
d81ae18
slice_sample_error class no longer used
romainfrancois Oct 22, 2021
b751f08
rlang now reporting the generic instead of the method
romainfrancois Oct 25, 2021
b591313
fix_call() default should be caller_env()
romainfrancois Oct 25, 2021
e21ed7d
fix_call() retrieves the call if given an environment
romainfrancois Oct 25, 2021
2f38747
using fix_call() default
romainfrancois Oct 25, 2021
ab10631
fix fix_call() when used from methods.
romainfrancois Oct 25, 2021
61e9b4d
filter() passing down call environment rather than making a call("fil…
romainfrancois Oct 25, 2021
538d60f
no need to make call("select")
romainfrancois Oct 25, 2021
c4639d6
using local_error_call()
romainfrancois Oct 25, 2021
98b18b6
mutate_cols(error_call=)
romainfrancois Oct 25, 2021
dc69df5
slice_internal(error_call =
romainfrancois Oct 25, 2021
ceba512
case_when()
romainfrancois Oct 26, 2021
a3b291e
rows_*
romainfrancois Oct 26, 2021
2806404
is_compatible_data_frame() making bullets
romainfrancois Oct 26, 2021
afbc99f
group_map() and friends
romainfrancois Oct 26, 2021
e8ed14d
implicit call
romainfrancois Oct 26, 2021
9935682
should -> must, automatic call
romainfrancois Oct 26, 2021
78e2fc1
common_by() automatic call
romainfrancois Oct 26, 2021
140ed30
group_by() automatic error_call
romainfrancois Oct 26, 2021
52131a5
summarise_cols() not taking ..., but taking error_call =
romainfrancois Oct 26, 2021
214e42b
nest_by() automatic call
romainfrancois Oct 26, 2021
ed44855
recode() automatic call
romainfrancois Oct 26, 2021
d46dc32
add_computed_columns() using error_call rather than add-hoc mechanism
romainfrancois Oct 26, 2021
9d2e41a
arrange() call
romainfrancois Oct 26, 2021
c8598cc
sample_* call=
romainfrancois Oct 26, 2021
e627bd0
More error_call
romainfrancois Oct 26, 2021
52fc5dc
DataMask$new(error_call=)
romainfrancois Oct 26, 2021
573dbce
document()
romainfrancois Oct 26, 2021
ce52c0c
filter_expand(mask=)
romainfrancois Oct 26, 2021
097cab2
remove snapshot using utf8
romainfrancois Oct 26, 2021
ce7f5d9
Merge branch 'main' into chained-errors
romainfrancois Oct 28, 2021
dde6cd8
simplicy slice() by using a slice_impl() called by slice.data.frame()…
romainfrancois Oct 28, 2021
986a38f
also use slice_bullets() for parity
romainfrancois Oct 28, 2021
123311b
remove some unused functions
romainfrancois Oct 28, 2021
5cd2b16
slice_bullets.default
romainfrancois Oct 28, 2021
5882c89
"must" form
romainfrancois Oct 28, 2021
ff9c167
Capitalize
romainfrancois Oct 28, 2021
2b9b68c
simplify errors from new_grouped_df()
romainfrancois Oct 28, 2021
b30e832
must form
romainfrancois Oct 28, 2021
bbe4771
must form and wrap error from validate_grouped_df()
romainfrancois Oct 28, 2021
3d1c614
simpler
romainfrancois Oct 28, 2021
9ac37b7
explicit abort(call =) instead of local_error_call()
romainfrancois Nov 8, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions NAMESPACE
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ S3method(filter,data.frame)
S3method(filter,ts)
S3method(filter_,data.frame)
S3method(filter_,tbl_df)
S3method(filter_bullets,"dplyr:::filter_incompatible_size")
S3method(filter_bullets,"dplyr:::filter_incompatible_type")
S3method(filter_bullets,default)
S3method(format,src_local)
S3method(full_join,data.frame)
S3method(group_by,data.frame)
Expand Down Expand Up @@ -94,6 +97,10 @@ S3method(left_join,data.frame)
S3method(mutate,data.frame)
S3method(mutate_,data.frame)
S3method(mutate_,tbl_df)
S3method(mutate_bullets,"dplyr:::mutate_incompatible_size")
S3method(mutate_bullets,"dplyr:::mutate_mixed_null")
S3method(mutate_bullets,"dplyr:::mutate_not_vector")
S3method(mutate_bullets,default)
S3method(n_groups,data.frame)
S3method(nest_by,data.frame)
S3method(nest_by,grouped_df)
Expand Down Expand Up @@ -137,6 +144,7 @@ S3method(setequal,data.frame)
S3method(slice,data.frame)
S3method(slice_,data.frame)
S3method(slice_,tbl_df)
S3method(slice_bullets,default)
S3method(slice_head,data.frame)
S3method(slice_max,data.frame)
S3method(slice_min,data.frame)
Expand All @@ -148,6 +156,11 @@ S3method(summarise,grouped_df)
S3method(summarise,rowwise_df)
S3method(summarise_,data.frame)
S3method(summarise_,tbl_df)
S3method(summarise_bullets,"dplyr:::error_summarise_incompatible_combine")
S3method(summarise_bullets,"dplyr:::summarise_incompatible_size")
S3method(summarise_bullets,"dplyr:::summarise_mixed_null")
S3method(summarise_bullets,"dplyr:::summarise_unsupported_type")
S3method(summarise_bullets,default)
S3method(tally,data.frame)
S3method(tbl,DBIConnection)
S3method(tbl,src_local)
Expand Down
73 changes: 45 additions & 28 deletions R/across.R
Original file line number Diff line number Diff line change
Expand Up @@ -172,18 +172,25 @@ across <- function(.cols = everything(), .fns = NULL, ..., .names = NULL) {

# Loop in such an order that all functions are applied
# to a single column before moving on to the next column
for (i in seq_n_cols) {
var <- vars[[i]]
col <- data[[i]]

context_poke("column", var)

for (j in seq_fns) {
fn <- fns[[j]]
out[[k]] <- fn(col, ...)
k <- k + 1L
withCallingHandlers(
for (i in seq_n_cols) {
var <- vars[[i]]
col <- data[[i]]

context_poke("column", var)

for (j in seq_fns) {
fn <- fns[[j]]
out[[k]] <- fn(col, ...)
k <- k + 1L
}
}, error = function(cnd) {
bullets <- c(
glue("Problem while computing column `{names[k]}`.")
)
abort(bullets, call = call(setup$across_if_fn), parent = cnd)
}
}
)

size <- vec_size_common(!!!out)
out <- vec_recycle_common(!!!out, .size = size)
Expand Down Expand Up @@ -247,7 +254,7 @@ c_across <- function(cols = everything()) {
cols <- enquo(cols)
vars <- c_across_setup(!!cols)

mask <- peek_mask("c_across()")
mask <- peek_mask("c_across")

cols <- mask$current_cols(vars)
vec_c(!!!cols, .name_spec = zap())
Expand All @@ -266,35 +273,43 @@ across_setup <- function(cols,
fns,
names,
.caller_env,
mask = peek_mask("across()"),
mask = peek_mask("across"),
inline = FALSE) {
cols <- enquo(cols)

# `across()` is evaluated in a data mask so we need to remove the
# mask layer from the quosure environment (#5460)
cols <- quo_set_env(cols, data_mask_top(quo_get_env(cols), recursive = FALSE, inherit = FALSE))

across_if_fn <- context_peek_bare("across_if_fn") %||% "across"

# TODO: call eval_select with a calling handler to intercept
# classed error, after https://github.com/r-lib/tidyselect/issues/233
if (is.null(fns) && quo_is_call(cols, "~")) {
if_fn <- context_peek_bare("across_if_fn") %||% "across"
abort(c(
"Predicate used in lieu of column selection.",
i = glue("You most likely meant: `{if_fn}(everything(), {as_label(cols)})`."),
bullets <- c(
"Must supply a column selection.",
i = glue("You most likely meant: `{across_if_fn}(everything(), {as_label(cols)})`."),
i = "The first argument `.cols` selects a set of columns.",
i = "The second argument `.fns` operates on each selected columns."
))
)
abort(bullets, call = call(across_if_fn))
}
across_cols <- mask$across_cols()

vars <- tidyselect::eval_select(cols, data = across_cols)
vars <- fix_call(
tidyselect::eval_select(cols, data = across_cols),
call = call(across_if_fn)
)
names_vars <- names(vars)
vars <- names(across_cols)[vars]

if (is.null(fns)) {
if (!is.null(names)) {
glue_mask <- across_glue_mask(.caller_env, .col = names_vars, .fn = "1")
names <- vec_as_names(glue(names, .envir = glue_mask), repair = "check_unique")
names <- fix_call(
vec_as_names(glue(names, .envir = glue_mask), repair = "check_unique"),
call = call(across_if_fn)
)
} else {
names <- names_vars
}
Expand All @@ -312,9 +327,8 @@ across_setup <- function(cols,
}

if (!is.list(fns)) {
abort(c("Problem with `across()` input `.fns`.",
i = "`.fns` must be NULL, a function, a formula, or a list of functions/formulas."
))
msg <- c("`.fns` must be NULL, a function, a formula, or a list of functions/formulas.")
abort(msg, call = call(across_if_fn))
}

# make sure fns has names, use number to replace unnamed
Expand All @@ -328,17 +342,20 @@ across_setup <- function(cols,
}
}

glue_mask <- glue_mask <- across_glue_mask(.caller_env,
glue_mask <- across_glue_mask(.caller_env,
.col = rep(names_vars, each = length(fns)),
.fn = rep(names_fns , length(vars))
)
names <- vec_as_names(glue(names, .envir = glue_mask), repair = "check_unique")
names <- fix_call(
vec_as_names(glue(names, .envir = glue_mask), repair = "check_unique"),
call = call(across_if_fn)
)

if (!inline) {
fns <- map(fns, as_function)
}

list(vars = vars, fns = fns, names = names)
list(vars = vars, fns = fns, names = names, across_if_fn = across_if_fn)
}

# FIXME: This pattern should be encapsulated by rlang
Expand All @@ -354,7 +371,7 @@ data_mask_top <- function(env, recursive = FALSE, inherit = FALSE) {
}

c_across_setup <- function(cols) {
mask <- peek_mask("c_across()")
mask <- peek_mask("c_across")

cols <- enquo(cols)
across_cols <- mask$across_cols()
Expand Down Expand Up @@ -528,7 +545,7 @@ expand_across <- function(quo) {
seq_vars <- seq_len(n_vars)
seq_fns <- seq_len(n_fns)

expressions <- vector(mode = "list", n_vars * n_fns)
expressions <- structure(vector(mode = "list", n_vars * n_fns), class = "dplyr_expanded_quosures")
columns <- character(n_vars * n_fns)

k <- 1L
Expand Down
6 changes: 3 additions & 3 deletions R/all-equal.r
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
#' all_equal(df1, df2, convert = TRUE)
all_equal <- function(target, current, ignore_col_order = TRUE,
ignore_row_order = TRUE, convert = FALSE, ...) {

equal_data_frame(target, current,
ignore_col_order = ignore_col_order,
ignore_row_order = ignore_row_order,
Expand All @@ -45,13 +44,14 @@ all_equal <- function(target, current, ignore_col_order = TRUE,
equal_data_frame <- function(x, y, ignore_col_order = TRUE, ignore_row_order = TRUE, convert = FALSE) {
compat <- is_compatible_data_frame(x, y, ignore_col_order = ignore_col_order, convert = convert)
if (!isTRUE(compat)) {
return(compat)
# revert the bulleting from is_compatible_data_frame()
return(glue_collapse(compat, sep = "\n"))
}

nrows_x <- nrow(x)
nrows_y <- nrow(y)
if (nrows_x != nrows_y) {
return("Different number of rows")
return("Different number of rows.")
}

if (ncol(x) == 0L) {
Expand Down
18 changes: 12 additions & 6 deletions R/arrange.R
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ arrange.data.frame <- function(.data, ..., .by_group = FALSE) {

# Helpers -----------------------------------------------------------------

arrange_rows <- function(.data, dots) {
arrange_rows <- function(.data, dots, error_call = caller_env()) {
if (length(dots) == 0L) {
out <- seq_len(nrow(.data))
return(out)
Expand All @@ -97,7 +97,7 @@ arrange_rows <- function(.data, dots) {
if (quo_is_call(quosure, "desc", ns = c("", "dplyr"))) {
expr <- quo_get_expr(quosure)
if (!has_length(expr, 2L)) {
abort("`desc()` expects exactly one argument.")
abort("`desc()` must be called with exactly one argument.", call = error_call)
}

quosure <- new_quosure(node_cadr(expr), quo_get_env(quosure))
Expand Down Expand Up @@ -126,14 +126,20 @@ arrange_rows <- function(.data, dots) {
names <- names2(bullets)
names[names == ""] <- "x"
bullets <- set_names(bullets, names)

# skip the parent as this has reworked the bullets
# and this would be confusing to have them
parent <- cnd$parent
} else {
bullets <- c(x = conditionMessage(cnd))
parent <- cnd
bullets <- c()
}

abort(c(
"arrange() failed at implicit mutate() step. ",
bullets <- c(
"Problem with the implicit `mutate()` step. ",
bullets
), class = "dplyr_error")
)
abort(bullets, call = error_call, parent = parent)

})

Expand Down
15 changes: 8 additions & 7 deletions R/bind.r
Original file line number Diff line number Diff line change
Expand Up @@ -101,19 +101,20 @@ bind_rows <- function(..., .id = NULL) {
for (i in seq_along(dots)) {
.x <- dots[[i]]
if (!is.data.frame(.x) && !vec_is(.x)) {
abort(glue("Argument {i} must be a data frame or a named atomic vector."))
msg <- glue("Argument {i} must be a data frame or a named atomic vector.")
abort(msg)
}

if (is.null(names(.x))) {
abort(glue("Argument {i} must have names."))
msg <- glue("Argument {i} must have names.")
abort(msg)
}
}

if (!is_null(.id)) {
if (!is_string(.id)) {
bad_args(".id", "must be a scalar string, ",
"not {friendly_type_of(.id)} of length {length(.id)}."
)
msg <- glue("`.id` must be a scalar string, not {friendly_type_of(.id)} of length {length(.id)}.")
abort(msg)
}
if (!is_named(dots)) {
names(dots) <- seq_along(dots)
Expand All @@ -135,7 +136,7 @@ bind_rows <- function(..., .id = NULL) {
if (is.null(.id)) {
names(dots) <- NULL
}
out <- vec_rbind(!!!dots, .names_to = .id)
out <- fix_call(vec_rbind(!!!dots, .names_to = .id))
if (length(dots)) {
if (is.data.frame(first)) {
out <- dplyr_reconstruct(out, first)
Expand All @@ -159,7 +160,7 @@ bind_cols <- function(..., .name_repair = c("unique", "universal", "check_unique
names[map_lgl(dots, is.data.frame)] <- ""
names(dots) <- names

out <- vec_cbind(!!!dots, .name_repair = .name_repair)
out <- fix_call(vec_cbind(!!!dots, .name_repair = .name_repair))
if (!any(map_lgl(dots, is.data.frame))) {
out <- as_tibble(out)
}
Expand Down
Loading