Skip to content

Commit

Permalink
Merge pull request #527 from yjunechoe/tidyselect-snapshot-test
Browse files Browse the repository at this point in the history
relax tests on tidyselect error messages
  • Loading branch information
rich-iannone committed Mar 11, 2024
2 parents e66acc7 + 175a1dd commit a57780f
Show file tree
Hide file tree
Showing 4 changed files with 133 additions and 128 deletions.
110 changes: 110 additions & 0 deletions R/utils-tidyselect.R
@@ -0,0 +1,110 @@
resolve_columns <- function(x, var_expr, preconditions = NULL, ...,
call = rlang::caller_env()) {

tbl <- apply_preconditions_for_cols(x, preconditions)

out <- tryCatch(
expr = resolve_columns_internal(tbl, var_expr, ..., call = call),
error = function(cnd) cnd
)

if (rlang::is_error(out)) {
# If error is a genuine evaluation error, throw that error
if (!is_subscript_error(out)) {
rlang::cnd_signal(rlang::error_cnd("resolve_eval_err", parent = out))
}
# If not in validation-planning context (assert/expect/test), rethrow
if (is_a_table_object(x) || is_secret_agent(x)) {
rlang::cnd_signal(out)
} else {
# Else (mid-planning): grab columns attempted to subset
fail <- out$i %||% out$parent$i
# If failure is due to scoping a bad object in the env, re-throw error
if (!is.character(fail) && !rlang::is_integerish(fail)) {
rlang::cnd_signal(out)
}
success <- resolve_columns_possible(tbl, var_expr)
out <- c(success, fail) %||% NA_character_
}
}

out

}

# Apply the preconditions function and resolve to data frame for tidyselect
apply_preconditions_for_cols <- function(x, preconditions) {
# Extract tbl
tbl <- if (is_ptblank_agent(x)) {
get_tbl_object(x)
} else if (is_a_table_object(x)) {
x
}
# Apply preconditions
if (!is.null(preconditions)) {
tbl <- apply_preconditions(tbl = tbl, preconditions = preconditions)
}
tbl
}

# Determines whether the error from a tidyselect expression is from attempting
# to select a non-existing column (i.e., a "subscript error")
is_subscript_error <- function(cnd) {
is.null(cnd$parent) || inherits(cnd$parent, "vctrs_error_subscript")
}

# If selection gets short-circuited by error, re-run with `strict = FALSE`
# to safely get the possible column selections
resolve_columns_possible <- function(tbl, var_expr) {
success <- tryCatch(
names(tidyselect::eval_select(var_expr, tbl,
strict = FALSE, allow_empty = FALSE)),
error = function(cnd) NULL
)
success
}

# Resolve column selections to integer
resolve_columns_internal <- function(tbl, var_expr, ..., call) {

# Return NA if the expr is NULL
if (rlang::quo_is_null(var_expr)) {
return(NA_character_)
}

# Special case `serially()`: just deparse elements and bypass tidyselect
if (rlang::is_empty(tbl)) {
var_expr <- rlang::quo_get_expr(var_expr)
if (rlang::is_symbol(var_expr) || rlang::is_scalar_character(var_expr)) {
column <- rlang::as_name(var_expr)
} else {
cols <- rlang::call_args(var_expr)
column <- vapply(cols, rlang::as_name, character(1), USE.NAMES = FALSE)
}
return(column)
}
# Special case `vars()`-expression for backwards compatibility
if (rlang::quo_is_call(var_expr, "vars")) {
var_expr <- rlang::quo_set_expr(var_expr, vars_to_c(var_expr))
}

# Proceed with tidyselect
column <- tidyselect::eval_select(var_expr, tbl, error_call = call, ...)
column <- names(column)

if (length(column) < 1) {
column <- NA_character_
}

column
}

# Convert to the idiomatic `c()`-expr before passing off to tidyselect
# + ensure that vars() always scopes symbols to data (vars(a) -> c("a"))
vars_to_c <- function(var_expr) {
var_args <- lapply(rlang::call_args(var_expr), function(var_arg) {
if (rlang::is_symbol(var_arg)) rlang::as_name(var_arg) else var_arg
})
c_expr <- rlang::call2("c", !!!var_args)
c_expr
}
103 changes: 0 additions & 103 deletions R/utils.R
Expand Up @@ -224,109 +224,6 @@ is_secret_agent <- function(x) {
is_ptblank_agent(x) && (x$label == "::QUIET::")
}

apply_preconditions_for_cols <- function(x, preconditions) {
# Extract tbl
tbl <- if (is_ptblank_agent(x)) {
get_tbl_object(x)
} else if (is_a_table_object(x)) {
x
}
# Apply preconditions
if (!is.null(preconditions)) {
tbl <- apply_preconditions(tbl = tbl, preconditions = preconditions)
}
tbl
}

resolve_columns <- function(x, var_expr, preconditions = NULL, ...,
call = rlang::caller_env()) {

tbl <- apply_preconditions_for_cols(x, preconditions)

out <- tryCatch(
expr = resolve_columns_internal(tbl, var_expr, ..., call = call),
error = function(cnd) cnd
)

if (rlang::is_error(out)) {
# If error is a genuine evaluation error, throw that error
if (!is.null(out$parent)) {
rlang::cnd_signal(rlang::error_cnd("resolve_eval_err", parent = out))
}
# If not in validation-planning context (assert/expect/test), rethrow
if (is_a_table_object(x) || is_secret_agent(x)) {
rlang::cnd_signal(out)
} else {
# Else (mid-planning): grab columns attempted to subset
fail <- out$i
# If failure is due to scoping a bad object in the env, re-throw error
if (!is.character(fail) && !rlang::is_integerish(fail)) {
rlang::cnd_signal(out)
}
success <- resolve_columns_possible(tbl, var_expr)
out <- c(success, fail) %||% NA_character_
}
}

out

}

# If selection gets short-circuited by error, re-run with `strict = FALSE`
# to safely get the possible column selections
resolve_columns_possible <- function(tbl, var_expr) {
success <- tryCatch(
names(tidyselect::eval_select(var_expr, tbl,
strict = FALSE, allow_empty = FALSE)),
error = function(cnd) NULL
)
success
}

resolve_columns_internal <- function(tbl, var_expr, ..., call) {

# Return NA if the expr is NULL
if (rlang::quo_is_null(var_expr)) {
return(NA_character_)
}

# Special case `serially()`: just deparse elements and bypass tidyselect
if (rlang::is_empty(tbl)) {
var_expr <- rlang::quo_get_expr(var_expr)
if (rlang::is_symbol(var_expr) || rlang::is_scalar_character(var_expr)) {
column <- rlang::as_name(var_expr)
} else {
cols <- rlang::call_args(var_expr)
column <- vapply(cols, rlang::as_name, character(1), USE.NAMES = FALSE)
}
return(column)
}
# Special case `vars()`-expression for backwards compatibility
if (rlang::quo_is_call(var_expr, "vars")) {
var_expr <- rlang::quo_set_expr(var_expr, vars_to_c(var_expr))
}

# Proceed with tidyselect
column <- tidyselect::eval_select(var_expr, tbl, error_call = call, ...)
column <- names(column)

if (length(column) < 1) {
column <- NA_character_
}

column
}

# Convert to the idiomatic `c()`-expr before passing off to tidyselect
# + ensure that vars() always scopes symbols to data (vars(a) -> c("a"))
vars_to_c <- function(var_expr) {
var_args <- lapply(rlang::call_args(var_expr), function(var_arg) {
if (rlang::is_symbol(var_arg)) rlang::as_name(var_arg) else var_arg
})
c_expr <- rlang::call2("c", !!!var_args)
c_expr
}

resolve_label <- function(label, columns = "", segments = "") {
n_columns <- length(columns)
n_segments <- length(segments)
Expand Down
42 changes: 20 additions & 22 deletions tests/testthat/test-tidyselect_fails_safely.R
Expand Up @@ -88,25 +88,23 @@ test_that("tidyselecting 0 columns for rows_* functions = fail at interrogation"

test_that("tidyselect errors *are* immediate for assertion/expectation/test", {

mismatch_msg <- "Can't subset columns that don't exist."

# For validation steps are used on table
expect_error(small_table %>% col_vals_not_null(z), mismatch_msg)
expect_error(small_table %>% col_vals_not_null("z"), mismatch_msg)
expect_error(small_table %>% col_vals_not_null(all_of("z")), mismatch_msg)
expect_error(small_table %>% col_vals_not_null(all_of(nonexistent_col)), mismatch_msg)
# For validation steps used on table
expect_error(small_table %>% col_vals_not_null(z))
expect_error(small_table %>% col_vals_not_null("z"))
expect_error(small_table %>% col_vals_not_null(all_of("z")))
expect_error(small_table %>% col_vals_not_null(all_of(nonexistent_col)))

# For expectations
expect_error(small_table %>% expect_col_vals_not_null(z), mismatch_msg)
expect_error(small_table %>% expect_col_vals_not_null("z"), mismatch_msg)
expect_error(small_table %>% expect_col_vals_not_null(all_of("z")), mismatch_msg)
expect_error(small_table %>% expect_col_vals_not_null(all_of(nonexistent_col)), mismatch_msg)
expect_error(small_table %>% expect_col_vals_not_null(z))
expect_error(small_table %>% expect_col_vals_not_null("z"))
expect_error(small_table %>% expect_col_vals_not_null(all_of("z")))
expect_error(small_table %>% expect_col_vals_not_null(all_of(nonexistent_col)))

# For tests
expect_error(small_table %>% test_col_vals_not_null(z), mismatch_msg)
expect_error(small_table %>% test_col_vals_not_null("z"), mismatch_msg)
expect_error(small_table %>% test_col_vals_not_null(all_of("z")), mismatch_msg)
expect_error(small_table %>% test_col_vals_not_null(all_of(nonexistent_col)), mismatch_msg)
expect_error(small_table %>% test_col_vals_not_null(z))
expect_error(small_table %>% test_col_vals_not_null("z"))
expect_error(small_table %>% test_col_vals_not_null(all_of("z")))
expect_error(small_table %>% test_col_vals_not_null(all_of(nonexistent_col)))

})

Expand All @@ -118,7 +116,7 @@ test_that("tidyselect errors cannot be downgraded in assertion/expectation on ta
col_vals_not_null(a) %>%
col_vals_not_null(z, actions = warn_on_fail()) %>%
col_vals_not_null(b)
}, "Can't subset columns that don't exist.")
})

})

Expand All @@ -127,16 +125,16 @@ test_that("env scoping with bare symbol patterns", {
# `z` is external vector of valid column
z <- "a"
rlang::local_options(lifecycle_verbosity = "warning")
expect_warning({small_table %>% col_vals_not_null(z)}, "deprecated")
expect_warning(small_table %>% col_vals_not_null(z))
rlang::local_options(lifecycle_verbosity = "quiet")

# `z` is not character
z <- mtcars
rlang::local_options(lifecycle_verbosity = "quiet")
# c() and vars() both error, but different reasons
## c() scopes z in env and determines its invalid
expect_error({small_table %>% col_vals_not_null(c(z))}, "`z` must be numeric or character")
## vars() doesn't attempt to scope z in env at all
expect_error({small_table %>% col_vals_not_null(vars(z))}, "Column `z` doesn't exist")
## c() scopes z in env and determines its invalid ("must be numeric or character")
expect_error(small_table %>% col_vals_not_null(c(z)))
## vars() doesn't attempt to scope z in env at all ("doesn't exist")
expect_error(small_table %>% col_vals_not_null(vars(z)))

# Cleanup
z <- rlang::missing_arg()
Expand Down
6 changes: 3 additions & 3 deletions tests/testthat/test-tidyselect_fails_safely_batch.R
Expand Up @@ -173,9 +173,9 @@ test_that("Genuine evaluation errors are rethrown immediately (tested on a sampl

err_expr <- errs[[err_regex]]

expect_error(agent %>% col_vals_between({{ err_expr }}, 2, 5), err_regex)
expect_error(agent %>% rows_distinct({{ err_expr }}), err_regex)
expect_error(agent %>% col_exists({{ err_expr }}), err_regex)
expect_error(agent %>% col_vals_between({{ err_expr }}, 2, 5))
expect_error(agent %>% rows_distinct({{ err_expr }}))
expect_error(agent %>% col_exists({{ err_expr }}))

}

Expand Down

0 comments on commit a57780f

Please sign in to comment.