From ad19eb12bbf284e9e061d00fb889e6bc6bdabb18 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 18 Oct 2022 10:22:33 -0500 Subject: [PATCH 01/26] Add rlang check_ helpers --- R/compat-obj-type.R | 339 +++++++++++++++++++++++++++++ R/compat-types-check.R | 471 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 810 insertions(+) create mode 100644 R/compat-obj-type.R create mode 100644 R/compat-types-check.R diff --git a/R/compat-obj-type.R b/R/compat-obj-type.R new file mode 100644 index 000000000..d86dca51b --- /dev/null +++ b/R/compat-obj-type.R @@ -0,0 +1,339 @@ +# nocov start --- r-lib/rlang compat-obj-type +# +# Changelog +# ========= +# +# 2022-10-04: +# - `obj_type_friendly(value = TRUE)` now shows numeric scalars +# literally. +# - `stop_friendly_type()` now takes `show_value`, passed to +# `obj_type_friendly()` as the `value` argument. +# +# 2022-10-03: +# - Added `allow_na` and `allow_null` arguments. +# - `NULL` is now backticked. +# - Better friendly type for infinities and `NaN`. +# +# 2022-09-16: +# - Unprefixed usage of rlang functions with `rlang::` to +# avoid onLoad issues when called from rlang (#1482). +# +# 2022-08-11: +# - Prefixed usage of rlang functions with `rlang::`. +# +# 2022-06-22: +# - `friendly_type_of()` is now `obj_type_friendly()`. +# - Added `obj_type_oo()`. +# +# 2021-12-20: +# - Added support for scalar values and empty vectors. +# - Added `stop_input_type()` +# +# 2021-06-30: +# - Added support for missing arguments. +# +# 2021-04-19: +# - Added support for matrices and arrays (#141). +# - Added documentation. +# - Added changelog. + + +#' Return English-friendly type +#' @param x Any R object. +#' @param value Whether to describe the value of `x`. Special values +#' like `NA` or `""` are always described. +#' @param length Whether to mention the length of vectors and lists. +#' @return A string describing the type. Starts with an indefinite +#' article, e.g. "an integer vector". +#' @noRd +obj_type_friendly <- function(x, value = TRUE) { + if (is_missing(x)) { + return("absent") + } + + if (is.object(x)) { + if (inherits(x, "quosure")) { + type <- "quosure" + } else { + type <- paste(class(x), collapse = "/") + } + return(sprintf("a <%s> object", type)) + } + + if (!is_vector(x)) { + return(.rlang_as_friendly_type(typeof(x))) + } + + n_dim <- length(dim(x)) + + if (!n_dim) { + if (!is_list(x) && length(x) == 1) { + if (is_na(x)) { + return(switch( + typeof(x), + logical = "`NA`", + integer = "an integer `NA`", + double = + if (is.nan(x)) { + "`NaN`" + } else { + "a numeric `NA`" + }, + complex = "a complex `NA`", + character = "a character `NA`", + .rlang_stop_unexpected_typeof(x) + )) + } + + show_infinites <- function(x) { + if (x > 0) { + "`Inf`" + } else { + "`-Inf`" + } + } + str_encode <- function(x, width = 30, ...) { + if (nchar(x) > width) { + x <- substr(x, 1, width - 3) + x <- paste0(x, "...") + } + encodeString(x, ...) + } + + if (value) { + if (is.numeric(x) && is.infinite(x)) { + return(show_infinites(x)) + } + + if (is.numeric(x) || is.complex(x)) { + number <- as.character(round(x, 2)) + what <- if (is.complex(x)) "the complex number" else "the number" + return(paste(what, number)) + } + + return(switch( + typeof(x), + logical = if (x) "`TRUE`" else "`FALSE`", + character = { + what <- if (nzchar(x)) "the string" else "the empty string" + paste(what, str_encode(x, quote = "\"")) + }, + raw = paste("the raw value", as.character(x)), + .rlang_stop_unexpected_typeof(x) + )) + } + + return(switch( + typeof(x), + logical = "a logical value", + integer = "an integer", + double = if (is.infinite(x)) show_infinites(x) else "a number", + complex = "a complex number", + character = if (nzchar(x)) "a string" else "\"\"", + raw = "a raw value", + .rlang_stop_unexpected_typeof(x) + )) + } + + if (length(x) == 0) { + return(switch( + typeof(x), + logical = "an empty logical vector", + integer = "an empty integer vector", + double = "an empty numeric vector", + complex = "an empty complex vector", + character = "an empty character vector", + raw = "an empty raw vector", + list = "an empty list", + .rlang_stop_unexpected_typeof(x) + )) + } + } + + vec_type_friendly(x) +} + +vec_type_friendly <- function(x, length = FALSE) { + if (!is_vector(x)) { + abort("`x` must be a vector.") + } + type <- typeof(x) + n_dim <- length(dim(x)) + + add_length <- function(type) { + if (length && !n_dim) { + paste0(type, sprintf(" of length %s", length(x))) + } else { + type + } + } + + if (type == "list") { + if (n_dim < 2) { + return(add_length("a list")) + } else if (is.data.frame(x)) { + return("a data frame") + } else if (n_dim == 2) { + return("a list matrix") + } else { + return("a list array") + } + } + + type <- switch( + type, + logical = "a logical %s", + integer = "an integer %s", + numeric = , + double = "a double %s", + complex = "a complex %s", + character = "a character %s", + raw = "a raw %s", + type = paste0("a ", type, " %s") + ) + + if (n_dim < 2) { + kind <- "vector" + } else if (n_dim == 2) { + kind <- "matrix" + } else { + kind <- "array" + } + out <- sprintf(type, kind) + + if (n_dim >= 2) { + out + } else { + add_length(out) + } +} + +.rlang_as_friendly_type <- function(type) { + switch( + type, + + list = "a list", + + NULL = "`NULL`", + environment = "an environment", + externalptr = "a pointer", + weakref = "a weak reference", + S4 = "an S4 object", + + name = , + symbol = "a symbol", + language = "a call", + pairlist = "a pairlist node", + expression = "an expression vector", + + char = "an internal string", + promise = "an internal promise", + ... = "an internal dots object", + any = "an internal `any` object", + bytecode = "an internal bytecode object", + + primitive = , + builtin = , + special = "a primitive function", + closure = "a function", + + type + ) +} + +.rlang_stop_unexpected_typeof <- function(x, call = caller_env()) { + abort( + sprintf("Unexpected type <%s>.", typeof(x)), + call = call + ) +} + +#' Return OO type +#' @param x Any R object. +#' @return One of `"bare"` (for non-OO objects), `"S3"`, `"S4"`, +#' `"R6"`, or `"R7"`. +#' @noRd +obj_type_oo <- function(x) { + if (!is.object(x)) { + return("bare") + } + + class <- inherits(x, c("R6", "R7_object"), which = TRUE) + + if (class[[1]]) { + "R6" + } else if (class[[2]]) { + "R7" + } else if (isS4(x)) { + "S4" + } else { + "S3" + } +} + +#' @param x The object type which does not conform to `what`. Its +#' `obj_type_friendly()` is taken and mentioned in the error message. +#' @param what The friendly expected type as a string. Can be a +#' character vector of expected types, in which case the error +#' message mentions all of them in an "or" enumeration. +#' @param show_value Passed to `value` argument of `obj_type_friendly()`. +#' @param ... Arguments passed to [abort()]. +#' @inheritParams args_error_context +#' @noRd +stop_input_type <- function(x, + what, + ..., + allow_na = FALSE, + allow_null = FALSE, + show_value = TRUE, + arg = caller_arg(x), + call = caller_env()) { + # From compat-cli.R + cli <- env_get_list( + nms = c("format_arg", "format_code"), + last = topenv(), + default = function(x) sprintf("`%s`", x), + inherit = TRUE + ) + + if (allow_na) { + what <- c(what, cli$format_code("NA")) + } + if (allow_null) { + what <- c(what, cli$format_code("NULL")) + } + if (length(what)) { + what <- oxford_comma(what) + } + + message <- sprintf( + "%s must be %s, not %s.", + cli$format_arg(arg), + what, + obj_type_friendly(x, value = show_value) + ) + + abort(message, ..., call = call, arg = arg) +} + +oxford_comma <- function(chr, sep = ", ", final = "or") { + n <- length(chr) + + if (n < 2) { + return(chr) + } + + head <- chr[seq_len(n - 1)] + last <- chr[n] + + head <- paste(head, collapse = sep) + + # Write a or b. But a, b, or c. + if (n > 2) { + paste0(head, sep, final, " ", last) + } else { + paste0(head, " ", final, " ", last) + } +} + +# nocov end diff --git a/R/compat-types-check.R b/R/compat-types-check.R new file mode 100644 index 000000000..3689ca5f2 --- /dev/null +++ b/R/compat-types-check.R @@ -0,0 +1,471 @@ +# nocov start --- r-lib/rlang compat-types-check +# +# Dependencies +# ============ +# +# - compat-obj-type.R +# +# Changelog +# ========= +# +# 2022-10-07: +# - `check_number_whole()` and `_decimal()` no longer treat +# non-numeric types such as factors or dates as numbers. Numeric +# types are detected with `is.numeric()`. +# +# 2022-10-04: +# - Added `check_name()` that forbids the empty string. +# `check_string()` allows the empty string by default. +# +# 2022-09-28: +# - Removed `what` arguments. +# - Added `allow_na` and `allow_null` arguments. +# - Added `allow_decimal` and `allow_infinite` arguments. +# - Improved errors with absent arguments. +# +# +# 2022-09-16: +# - Unprefixed usage of rlang functions with `rlang::` to +# avoid onLoad issues when called from rlang (#1482). +# +# 2022-08-11: +# - Added changelog. + +# Scalars ----------------------------------------------------------------- + +check_bool <- function(x, + ..., + allow_na = FALSE, + allow_null = FALSE, + arg = caller_arg(x), + call = caller_env()) { + if (!missing(x)) { + if (is_bool(x)) { + return(invisible(NULL)) + } + if (allow_null && is_null(x)) { + return(invisible(NULL)) + } + if (allow_na && identical(x, NA)) { + return(invisible(NULL)) + } + } + + stop_input_type( + x, + c("`TRUE`", "`FALSE`"), + ..., + allow_na = allow_na, + allow_null = allow_null, + arg = arg, + call = call + ) +} + +check_string <- function(x, + ..., + allow_empty = TRUE, + allow_na = FALSE, + allow_null = FALSE, + arg = caller_arg(x), + call = caller_env()) { + if (!missing(x)) { + is_string <- .rlang_check_is_string( + x, + allow_empty = allow_empty, + allow_na = allow_na, + allow_null = allow_null + ) + if (is_string) { + return(invisible(NULL)) + } + } + + stop_input_type( + x, + "a single string", + ..., + allow_na = allow_na, + allow_null = allow_null, + arg = arg, + call = call + ) +} + +.rlang_check_is_string <- function(x, + allow_empty, + allow_na, + allow_null) { + if (is_string(x)) { + if (allow_empty || !is_string(x, "")) { + return(TRUE) + } + } + + if (allow_null && is_null(x)) { + return(TRUE) + } + + if (allow_na && (identical(x, NA) || identical(x, na_chr))) { + return(TRUE) + } + + FALSE +} + +check_name <- function(x, + ..., + allow_null = FALSE, + arg = caller_arg(x), + call = caller_env()) { + if (!missing(x)) { + is_string <- .rlang_check_is_string( + x, + allow_empty = FALSE, + allow_na = FALSE, + allow_null = allow_null + ) + if (is_string) { + return(invisible(NULL)) + } + } + + stop_input_type( + x, + "a valid name", + ..., + allow_na = FALSE, + allow_null = allow_null, + arg = arg, + call = call + ) +} + +check_number_decimal <- function(x, + ..., + min = -Inf, + max = Inf, + allow_infinite = TRUE, + allow_na = FALSE, + allow_null = FALSE, + arg = caller_arg(x), + call = caller_env()) { + .rlang_types_check_number( + x, + ..., + min = min, + max = max, + allow_decimal = TRUE, + allow_infinite = allow_infinite, + allow_na = allow_na, + allow_null = allow_null, + arg = arg, + call = call + ) +} + +check_number_whole <- function(x, + ..., + min = -Inf, + max = Inf, + allow_na = FALSE, + allow_null = FALSE, + arg = caller_arg(x), + call = caller_env()) { + .rlang_types_check_number( + x, + ..., + min = min, + max = max, + allow_decimal = FALSE, + allow_infinite = FALSE, + allow_na = allow_na, + allow_null = allow_null, + arg = arg, + call = call + ) +} + +.rlang_types_check_number <- function(x, + ..., + min = -Inf, + max = Inf, + allow_decimal = FALSE, + allow_infinite = FALSE, + allow_na = FALSE, + allow_null = FALSE, + arg = caller_arg(x), + call = caller_env()) { + if (allow_decimal) { + what <- "a number" + } else { + what <- "a whole number" + } + + .stop <- function(x, what, ...) stop_input_type( + x, + what, + ..., + allow_na = allow_na, + allow_null = allow_null, + arg = arg, + call = call + ) + + if (!missing(x)) { + is_number <- is_number( + x, + allow_decimal = allow_decimal, + allow_infinite = allow_infinite + ) + + if (is_number) { + if (min > -Inf && max < Inf) { + what <- sprintf("a number between %s and %s", min, max) + } else { + what <- NULL + } + if (x < min) { + what <- what %||% sprintf("a number larger than %s", min) + .stop(x, what, ...) + } + if (x > max) { + what <- what %||% sprintf("a number smaller than %s", max) + .stop(x, what, ...) + } + return(invisible(NULL)) + } + + if (allow_null && is_null(x)) { + return(invisible(NULL)) + } + if (allow_na && (identical(x, NA) || + identical(x, na_dbl) || + identical(x, na_int))) { + return(invisible(NULL)) + } + } + + .stop(x, what, ...) +} + +is_number <- function(x, + allow_decimal = FALSE, + allow_infinite = FALSE) { + if (!typeof(x) %in% c("integer", "double")) { + return(FALSE) + } + if (!is.numeric(x)) { + return(FALSE) + } + if (length(x) != 1) { + return(FALSE) + } + if (is.na(x)) { + return(FALSE) + } + if (!allow_decimal && !is_integerish(x)) { + return(FALSE) + } + if (!allow_infinite && is.infinite(x)) { + return(FALSE) + } + TRUE +} + +check_symbol <- function(x, + ..., + allow_null = FALSE, + arg = caller_arg(x), + call = caller_env()) { + if (!missing(x)) { + if (is_symbol(x)) { + return(invisible(NULL)) + } + if (allow_null && is_null(x)) { + return(invisible(NULL)) + } + } + + stop_input_type( + x, + "a symbol", + ..., + allow_null = allow_null, + arg = arg, + call = call + ) +} + +check_arg <- function(x, + ..., + allow_null = FALSE, + arg = caller_arg(x), + call = caller_env()) { + if (!missing(x)) { + if (is_symbol(x)) { + return(invisible(NULL)) + } + if (allow_null && is_null(x)) { + return(invisible(NULL)) + } + } + + stop_input_type( + x, + "an argument name", + ..., + allow_null = allow_null, + arg = arg, + call = call + ) +} + +check_call <- function(x, + ..., + allow_null = FALSE, + arg = caller_arg(x), + call = caller_env()) { + if (!missing(x)) { + if (is_call(x)) { + return(invisible(NULL)) + } + if (allow_null && is_null(x)) { + return(invisible(NULL)) + } + } + + stop_input_type( + x, + "a defused call", + ..., + allow_null = allow_null, + arg = arg, + call = call + ) +} + +check_environment <- function(x, + ..., + allow_null = FALSE, + arg = caller_arg(x), + call = caller_env()) { + if (!missing(x)) { + if (is_environment(x)) { + return(invisible(NULL)) + } + if (allow_null && is_null(x)) { + return(invisible(NULL)) + } + } + + stop_input_type( + x, + "an environment", + ..., + allow_null = allow_null, + arg = arg, + call = call + ) +} + +check_function <- function(x, + ..., + allow_null = FALSE, + arg = caller_arg(x), + call = caller_env()) { + if (!missing(x)) { + if (is_function(x)) { + return(invisible(NULL)) + } + if (allow_null && is_null(x)) { + return(invisible(NULL)) + } + } + + stop_input_type( + x, + "a function", + ..., + allow_null = allow_null, + arg = arg, + call = call + ) +} + +check_closure <- function(x, + ..., + allow_null = FALSE, + arg = caller_arg(x), + call = caller_env()) { + if (!missing(x)) { + if (is_closure(x)) { + return(invisible(NULL)) + } + if (allow_null && is_null(x)) { + return(invisible(NULL)) + } + } + + stop_input_type( + x, + "an R function", + ..., + allow_null = allow_null, + arg = arg, + call = call + ) +} + +check_formula <- function(x, + ..., + allow_null = FALSE, + arg = caller_arg(x), + call = caller_env()) { + if (!missing(x)) { + if (is_formula(x)) { + return(invisible(NULL)) + } + if (allow_null && is_null(x)) { + return(invisible(NULL)) + } + } + + stop_input_type( + x, + "a formula", + ..., + allow_null = allow_null, + arg = arg, + call = call + ) +} + + +# Vectors ----------------------------------------------------------------- + +check_character <- function(x, + ..., + allow_null = FALSE, + arg = caller_arg(x), + call = caller_env()) { + if (!missing(x)) { + if (is_character(x)) { + return(invisible(NULL)) + } + if (allow_null && is_null(x)) { + return(invisible(NULL)) + } + } + + stop_input_type( + x, + "a character vector", + ..., + allow_null = allow_null, + arg = arg, + call = call + ) +} + +# nocov end From 56fe3e63bb0ae212eb6f7d79e53e0649d67ca29e Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 18 Oct 2022 10:32:48 -0500 Subject: [PATCH 02/26] Improve chop/unchop input validation --- R/chop.R | 12 +++++------- R/utils.R | 6 ++++++ tests/testthat/_snaps/chop.md | 35 +++++++++++++++++++++++++++++++---- tests/testthat/test-chop.R | 18 ++++++++++++++++-- 4 files changed, 58 insertions(+), 13 deletions(-) diff --git a/R/chop.R b/R/chop.R index 888d67381..c6d6fbe17 100644 --- a/R/chop.R +++ b/R/chop.R @@ -66,6 +66,7 @@ #' df %>% unchop(y) #' df %>% unchop(y, keep_empty = TRUE) chop <- function(data, cols) { + check_data_frame(data) check_required(cols) cols <- tidyselect::eval_select(enquo(cols), data, allow_rename = FALSE) @@ -98,6 +99,10 @@ col_chop <- function(x, indices) { #' @export #' @rdname chop unchop <- function(data, cols, keep_empty = FALSE, ptype = NULL) { + check_data_frame(data) + check_required(cols) + check_bool(keep_empty) + sel <- tidyselect::eval_select(enquo(cols), data) size <- vec_size(data) @@ -143,13 +148,6 @@ unchop <- function(data, cols, keep_empty = FALSE, ptype = NULL) { df_unchop <- function(x, ..., ptype = NULL, keep_empty = FALSE, error_call = caller_env()) { check_dots_empty() - if (!is.data.frame(x)) { - abort("`x` must be a data frame.", call = error_call) - } - if (!is_bool(keep_empty)) { - abort("`keep_empty` must be a single `TRUE` or `FALSE`.", call = error_call) - } - ptype <- check_list_of_ptypes(ptype, names = names(x), arg = "ptype", call = error_call) size <- vec_size(x) diff --git a/R/utils.R b/R/utils.R index aeffda41b..f75a2ed31 100644 --- a/R/utils.R +++ b/R/utils.R @@ -193,6 +193,12 @@ vec_paste0 <- function(...) { exec(paste0, !!!args) } +check_data_frame <- function(x, arg = caller_arg(x), call = caller_env()) { + if (!is.data.frame(x)) { + cli::cli_abort("{.arg {arg}} must be a data frame, not {.obj_type_friendly x}.", call = call) + } +} + check_list_of_ptypes <- function(x, names, arg, call = caller_env()) { if (vec_is(x) && vec_is_empty(x)) { x <- rep_named(names, list(x)) diff --git a/tests/testthat/_snaps/chop.md b/tests/testthat/_snaps/chop.md index 4d74fe84f..21e30ea8a 100644 --- a/tests/testthat/_snaps/chop.md +++ b/tests/testthat/_snaps/chop.md @@ -1,9 +1,13 @@ -# `cols` is required (#1205) +# chop() validates its input `cols` (#1205) Code - (expect_error(chop(df))) - Output - + chop(df$x) + Condition + Error in `chop()`: + ! `data` must be a data frame, not a string. + Code + chop(df) + Condition Error in `chop()`: ! `cols` is absent but must be supplied. @@ -25,3 +29,26 @@ Error in `unchop()`: ! In row 1, can't recycle input of size 0 to size 2. +# unchop validates its inputs + + Code + unchop(1:10) + Condition + Error in `unchop()`: + ! `data` must be a data frame, not a string. + Code + unchop(df) + Condition + Error in `unchop()`: + ! `cols` is absent but must be supplied. + Code + unchop(df, col, keep_empty = 1) + Condition + Error in `unchop()`: + ! `keep_empty` must be `TRUE` or `FALSE`, not the number 1. + Code + unchop(df, col, ptype = 1) + Condition + Error in `unchop()`: + ! `ptype` must be `NULL`, an empty ptype, or a named list of ptypes. + diff --git a/tests/testthat/test-chop.R b/tests/testthat/test-chop.R index b217bfb2a..a89fc2816 100644 --- a/tests/testthat/test-chop.R +++ b/tests/testthat/test-chop.R @@ -20,9 +20,12 @@ test_that("grouping is preserved", { expect_equal(dplyr::group_vars(out), "g") }) -test_that("`cols` is required (#1205)", { +test_that("chop() validates its input `cols` (#1205)", { df <- tibble(x = 1:2) - expect_snapshot((expect_error(chop(df)))) + expect_snapshot(error = TRUE, { + chop(df$x) + chop(df) + }) }) test_that("can chop empty data frame (#1206)", { @@ -305,3 +308,14 @@ test_that("unchopping drops outer names", { out <- unchop(df, col) expect_named(out$col, NULL) }) + +test_that("unchop validates its inputs", { + df <- tibble(col = list(a = 1, b = 2:3)) + + expect_snapshot(error = TRUE, { + unchop(1:10) + unchop(df) + unchop(df, col, keep_empty = 1) + unchop(df, col, ptype = 1) + }) +}) From baf48ef051d79913b38ad281eba3a8f9c7de5fa6 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 18 Oct 2022 10:33:06 -0500 Subject: [PATCH 03/26] Improve complete input validation --- R/complete.R | 7 +++---- tests/testthat/_snaps/complete.md | 8 ++++++++ tests/testthat/test-complete.R | 7 +++++++ 3 files changed, 18 insertions(+), 4 deletions(-) create mode 100644 tests/testthat/_snaps/complete.md diff --git a/R/complete.R b/R/complete.R index 715c11a85..36c2a2e16 100644 --- a/R/complete.R +++ b/R/complete.R @@ -64,6 +64,9 @@ complete <- function(data, ..., fill = list(), explicit = TRUE) { + + check_bool(explicit) + UseMethod("complete") } @@ -76,10 +79,6 @@ complete.data.frame <- function(data, ..., fill = list(), explicit = TRUE) { - if (!is_bool(explicit)) { - abort("`explicit` must be a single `TRUE` or `FALSE`.") - } - out <- expand(data, ...) names <- names(out) diff --git a/tests/testthat/_snaps/complete.md b/tests/testthat/_snaps/complete.md new file mode 100644 index 000000000..2b0f82ab9 --- /dev/null +++ b/tests/testthat/_snaps/complete.md @@ -0,0 +1,8 @@ +# validates its inputs + + Code + complete(mtcars, explicit = 1) + Condition + Error in `complete()`: + ! `explicit` must be `TRUE` or `FALSE`, not the number 1. + diff --git a/tests/testthat/test-complete.R b/tests/testthat/test-complete.R index 40881a8a8..28ba26a3d 100644 --- a/tests/testthat/test-complete.R +++ b/tests/testthat/test-complete.R @@ -195,3 +195,10 @@ test_that("if the completing variables have missings, `fill` will fill them afte tibble(x = c(1, 1, NA, 0), y = c(1, NA, 1, 0)) ) }) + + +test_that("validates its inputs", { + expect_snapshot(error = TRUE, { + complete(mtcars, explicit = 1) + }) +}) From 610999c3878eed7eabc748cd1860a26749e15005 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 18 Oct 2022 10:39:19 -0500 Subject: [PATCH 04/26] Improve extract() input validation --- R/extract.R | 15 +++++++-------- R/separate.R | 8 +++----- tests/testthat/_snaps/extract.md | 18 ++++++++++++++++++ tests/testthat/test-extract.R | 11 +++++++++++ 4 files changed, 39 insertions(+), 13 deletions(-) diff --git a/R/extract.R b/R/extract.R index f59627faa..a21fb1a3e 100644 --- a/R/extract.R +++ b/R/extract.R @@ -42,6 +42,7 @@ extract <- function(data, col, into, regex = "([[:alnum:]]+)", extract.data.frame <- function(data, col, into, regex = "([[:alnum:]]+)", remove = TRUE, convert = FALSE, ...) { check_required(col) + var <- tidyselect::vars_pull(names(data), !!enquo(col)) value <- as.character(data[[var]]) @@ -51,17 +52,15 @@ extract.data.frame <- function(data, col, into, regex = "([[:alnum:]]+)", } str_extract <- function(x, into, regex, convert = FALSE, error_call = caller_env()) { - check_not_stringr_pattern(regex, "regex", call = error_call) - - stopifnot( - is_string(regex), - is_character(into) - ) + check_string(regex, call = error_call) + check_not_stringr_pattern(regex, call = error_call) + check_character(into, call = error_call) + check_bool(convert, call = error_call) out <- str_match_first(x, regex) if (length(out) != length(into)) { - abort( - glue("`regex` should define {length(into)} groups; {length(out)} found."), + cli::cli_abort( + "{.arg regex} should define {length(into)} groups; {length(out)} found.", call = error_call ) } diff --git a/R/separate.R b/R/separate.R index bb56904b0..c07cd40bf 100644 --- a/R/separate.R +++ b/R/separate.R @@ -87,7 +87,7 @@ separate.data.frame <- function(data, col, into, sep = "[^[:alnum:]]+", } str_separate <- function(x, into, sep, convert = FALSE, extra = "warn", fill = "warn", error_call = caller_env()) { - check_not_stringr_pattern(sep, "sep", call = error_call) + check_not_stringr_pattern(sep, call = error_call) if (!is.character(into)) { abort("`into` must be a character vector.", call = error_call) @@ -186,10 +186,8 @@ list_indices <- function(x, max = 20) { paste(x, collapse = ", ") } -check_not_stringr_pattern <- function(x, arg, call = caller_env()) { +check_not_stringr_pattern <- function(x, arg = caller_arg(x), call = caller_env()) { if (inherits_any(x, c("pattern", "stringr_pattern"))) { - abort(glue("`{arg}` can't use modifiers from stringr."), call = call) + cli::cli_abort("{.arg {arg}} can't use modifiers from stringr.", call = call) } - - invisible(x) } diff --git a/tests/testthat/_snaps/extract.md b/tests/testthat/_snaps/extract.md index 30754202c..dd62652a2 100644 --- a/tests/testthat/_snaps/extract.md +++ b/tests/testthat/_snaps/extract.md @@ -22,3 +22,21 @@ Error in `extract()`: ! `regex` can't use modifiers from stringr. +# validates its inputs + + Code + df %>% extract() + Condition + Error in `extract()`: + ! `col` is absent but must be supplied. + Code + df %>% extract(x, regex = 1) + Condition + Error in `extract()`: + ! `regex` must be a single string, not the number 1. + Code + df %>% extract(x, into = 1:3) + Condition + Error in `extract()`: + ! `into` must be a character vector, not an integer vector. + diff --git a/tests/testthat/test-extract.R b/tests/testthat/test-extract.R index 2994b118f..ca87632ae 100644 --- a/tests/testthat/test-extract.R +++ b/tests/testthat/test-extract.R @@ -82,3 +82,14 @@ test_that("str_match_first handles edge cases", { list(character(), character()) ) }) + +test_that("validates its inputs", { + df <- data.frame(x = letters) + + expect_snapshot(error = TRUE, { + df %>% extract() + df %>% extract(x, regex = 1) + df %>% extract(x, into = 1:3) + df %>% extract(x, into = "x", convert = 1) + }) +}) From 7b8258dd3adf4a2c3fc6a8a75689f704631793d4 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 18 Oct 2022 10:41:21 -0500 Subject: [PATCH 05/26] Tweak fill input validation --- R/fill.R | 1 - tests/testthat/_snaps/fill.md | 8 ++++++++ tests/testthat/test-fill.R | 7 +++++++ 3 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 tests/testthat/_snaps/fill.md diff --git a/R/fill.R b/R/fill.R index e206dbe50..69b16d03e 100644 --- a/R/fill.R +++ b/R/fill.R @@ -95,7 +95,6 @@ fill.data.frame <- function(data, ..., .direction = c("down", "up", "downup", "u .direction <- arg_match0( arg = .direction, values = c("down", "up", "downup", "updown"), - arg_nm = ".direction" ) fn <- function(col) { diff --git a/tests/testthat/_snaps/fill.md b/tests/testthat/_snaps/fill.md new file mode 100644 index 000000000..b4ba7c64c --- /dev/null +++ b/tests/testthat/_snaps/fill.md @@ -0,0 +1,8 @@ +# validates its inputs + + Code + df %>% fill(x, .direction = "foo") + Condition + Error in `fill()`: + ! `.direction` must be one of "down", "up", "downup", or "updown", not "foo". + diff --git a/tests/testthat/test-fill.R b/tests/testthat/test-fill.R index d1b38ae10..0c0279b99 100644 --- a/tests/testthat/test-fill.R +++ b/tests/testthat/test-fill.R @@ -119,3 +119,10 @@ test_that("works when there is a column named `.direction` in the data (#1319)", expect_error(out <- fill(df, x), NA) expect_identical(out$x, c(1, 1, 2)) }) + +test_that("validates its inputs", { + df <- tibble(x = c(1, NA, 2)) + expect_snapshot(error = TRUE, { + df %>% fill(x, .direction = "foo") + }) +}) From b4e9713a2be6283587438f639483d3639dd81f09 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 18 Oct 2022 11:21:39 -0500 Subject: [PATCH 06/26] Add missing snapshot --- tests/testthat/_snaps/extract.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/testthat/_snaps/extract.md b/tests/testthat/_snaps/extract.md index dd62652a2..bb8f779ed 100644 --- a/tests/testthat/_snaps/extract.md +++ b/tests/testthat/_snaps/extract.md @@ -39,4 +39,9 @@ Condition Error in `extract()`: ! `into` must be a character vector, not an integer vector. + Code + df %>% extract(x, into = "x", convert = 1) + Condition + Error in `extract()`: + ! `convert` must be `TRUE` or `FALSE`, not the number 1. From c681020116ec3d95057e58d298c7cdff4e599de3 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 18 Oct 2022 11:28:31 -0500 Subject: [PATCH 07/26] Improve hoist input validation * Add new `check_unique_names()` * Refactor and update `check_list_of_ptypes()` and `check_list_of_functions()` * Add matching `check_list_of_bool()` --- R/hoist.R | 27 ++++------ R/pivot-long.R | 4 +- R/pivot-wide.R | 4 +- R/unnest-helper.R | 26 ++------- R/utils.R | 75 ++++++++++++++------------ tests/testthat/_snaps/hoist.md | 38 ++++++++++--- tests/testthat/_snaps/pivot-long.md | 4 +- tests/testthat/_snaps/pivot-wide.md | 4 +- tests/testthat/_snaps/unnest-helper.md | 2 +- tests/testthat/test-hoist.R | 13 ++++- 10 files changed, 108 insertions(+), 89 deletions(-) diff --git a/R/hoist.R b/R/hoist.R index 68795ed91..6cd881885 100644 --- a/R/hoist.R +++ b/R/hoist.R @@ -79,19 +79,20 @@ hoist <- function(.data, .simplify = TRUE, .ptype = NULL, .transform = NULL) { - if (!is.data.frame(.data)) { - abort("`.data` must be a data frame.") - } + check_data_frame(.data) check_required(.col) - .col <- tidyselect::vars_pull(names(.data), {{ .col }}) + pluckers <- check_pluckers(...) + check_bool(.remove) + .col <- tidyselect::vars_pull(names(.data), {{ .col }}) x <- .data[[.col]] - if (!vec_is_list(x)) { - abort("`.col` must identify a list-column.") - } + vec_check_list(x, arg = ".data[[.col]]") - pluckers <- check_pluckers(...) + # Check these here to generate error message with correct variable name + check_list_of_ptypes(.ptype, names(x)) + check_list_of_functions(.transform, names(x)) + check_list_of_bool(.simplify, names(x)) # In R <4.1, `::` is quite slow and this is a tight loop, so eliminating # the lookup has a large performance impact: @@ -143,13 +144,7 @@ check_pluckers <- function(..., .call = caller_env()) { names(pluckers)[auto_name] <- unlist(pluckers[auto_name]) } - if (length(pluckers) > 0 && !is_named(pluckers)) { - abort("All elements of `...` must be named.", call = .call) - } - - if (vec_duplicate_any(names(pluckers))) { - abort("The names of `...` must be unique.", call = .call) - } + check_unique_names(pluckers, arg = "...", call = .call) # Standardize all pluckers to lists for splicing into `pluck()` # and for easier handling in `strike()` @@ -161,7 +156,7 @@ check_pluckers <- function(..., .call = caller_env()) { strike <- function(x, indices) { if (!vec_is_list(indices)) { - abort("Internal error: `indices` must be a list.") + abort("`indices` must be a list.", .internal = TRUE) } n_indices <- vec_size(indices) diff --git a/R/pivot-long.R b/R/pivot-long.R index 0416b57e9..be0a3b841 100644 --- a/R/pivot-long.R +++ b/R/pivot-long.R @@ -263,8 +263,8 @@ pivot_longer_spec <- function(data, # TODO: Remove me after https://github.com/tidyverse/tidyr/issues/1296 values_ptypes <- NULL } - values_ptypes <- check_list_of_ptypes(values_ptypes, value_names, "values_ptypes") - values_transform <- check_list_of_functions(values_transform, value_names, "values_transform") + values_ptypes <- check_list_of_ptypes(values_ptypes, value_names) + values_transform <- check_list_of_functions(values_transform, value_names) vals <- set_names(vec_init(list(), length(values)), value_names) for (value in value_names) { diff --git a/R/pivot-wide.R b/R/pivot-wide.R index 0e47f9b1c..ce0035637 100644 --- a/R/pivot-wide.R +++ b/R/pivot-wide.R @@ -299,10 +299,10 @@ pivot_wider_spec <- function(data, values_from_cols = values_from_cols ) - values_fn <- check_list_of_functions(values_fn, values_from_cols, "values_fn") + values_fn <- check_list_of_functions(values_fn, values_from_cols) unused_cols <- setdiff(names(data), c(id_cols, names_from_cols, values_from_cols)) - unused_fn <- check_list_of_functions(unused_fn, unused_cols, "unused_fn") + unused_fn <- check_list_of_functions(unused_fn, unused_cols) unused_cols <- names(unused_fn) if (is.null(values_fill)) { diff --git a/R/unnest-helper.R b/R/unnest-helper.R index 881b362d1..04d94a1d1 100644 --- a/R/unnest-helper.R +++ b/R/unnest-helper.R @@ -9,25 +9,9 @@ df_simplify <- function(x, error_call = caller_env()) { check_dots_empty() - ptype <- check_list_of_ptypes(ptype, names(x), "ptype", call = error_call) - transform <- check_list_of_functions(transform, names(x), "transform", call = error_call) - - if (is_bool(simplify)) { - simplify_default <- simplify - simplify <- list() - } else { - simplify_default <- TRUE - } - - if (!vec_is_list(simplify)) { - abort("`simplify` must be a list or a single `TRUE` or `FALSE`.", call = error_call) - } - if (length(simplify) > 0L && !is_named(simplify)) { - abort("All elements of `simplify` must be named.", call = error_call) - } - if (vec_duplicate_any(names(simplify))) { - abort("The names of `simplify` must be unique.", call = error_call) - } + ptype <- check_list_of_ptypes(ptype, names(x), call = error_call) + transform <- check_list_of_functions(transform, names(x), call = error_call) + simplify <- check_list_of_bool(simplify, names(x), call = error_call) x_n <- length(x) x_size <- vec_size(x) @@ -42,13 +26,13 @@ df_simplify <- function(x, col_ptype <- ptype[[col_name]] col_transform <- transform[[col_name]] - col_simplify <- simplify[[col_name]] %||% simplify_default + col_simplify <- simplify[[col_name]] out[[i]] <- col_simplify( x = col, ptype = col_ptype, transform = col_transform, - simplify = col_simplify, + simplify = col_simplify %||% TRUE, error_call = error_call ) } diff --git a/R/utils.R b/R/utils.R index f75a2ed31..7f3a1f88a 100644 --- a/R/utils.R +++ b/R/utils.R @@ -199,57 +199,64 @@ check_data_frame <- function(x, arg = caller_arg(x), call = caller_env()) { } } -check_list_of_ptypes <- function(x, names, arg, call = caller_env()) { - if (vec_is(x) && vec_is_empty(x)) { - x <- rep_named(names, list(x)) +check_unique_names <- function(x, arg = caller_arg(x), call = caller_env()) { + if (length(x) > 0L && !is_named(x)) { + cli::cli_abort("All elements of {.arg {arg}} must be named.", call = call) } - - if (is.null(x)) { - x <- set_names(list(), character()) + if (vec_duplicate_any(names(x))) { + cli::cli_abort("The names of {.arg {arg}} must be unique.", call = call) } +} - if (!vec_is_list(x)) { - abort( - glue("`{arg}` must be `NULL`, an empty ptype, or a named list of ptypes."), +check_list_of_ptypes <- function(x, names, arg = caller_arg(x), call = caller_env()) { + if (is.null(x)) { + set_names(list(), character()) + } else if (vec_is(x) && vec_is_empty(x)) { + rep_named(names, list(x)) + } else if (vec_is_list(x)) { + check_unique_names(x, arg = arg, call = call) + + # Silently drop user supplied names not found in the data + x[intersect(names(x), names)] + } else { + cli::cli_abort( + "{.arg {arg}} must be `NULL`, an empty ptype, or a named list of ptypes.", call = call ) } - - if (length(x) > 0L && !is_named(x)) { - abort(glue("All elements of `{arg}` must be named."), call = call) - } - - if (vec_duplicate_any(names(x))) { - abort(glue("The names of `{arg}` must be unique."), call = call) - } - - # Silently drop user supplied names not found in the data - x <- x[intersect(names(x), names)] - - x } -check_list_of_functions <- function(x, names, arg, call = caller_env()) { +check_list_of_functions <- function(x, names, arg = caller_arg(x), call = caller_env()) { if (is.null(x)) { x <- set_names(list(), character()) - } - - if (!vec_is_list(x)) { + } else if (is.function(x) || is_formula(x)) { x <- rep_named(names, list(x)) + } else if (!vec_is_list(x)) { + cli::cli_abort( + "{.arg {arg}} must be `NULL`, a function, or a named list of functions.", + call = call + ) } - if (length(x) > 0L && !is_named(x)) { - abort(glue("All elements of `{arg}` must be named."), call = call) - } - - if (vec_duplicate_any(names(x))) { - abort(glue("The names of `{arg}` must be unique."), call = call) - } + check_unique_names(x, arg = arg, call = call) x <- map(x, as_function, arg = arg, call = call) - # Silently drop user supplied names not found in the data x <- x[intersect(names(x), names)] x } + +check_list_of_bool <- function(x, names, arg = caller_arg(x), call = caller_env()) { + if (is_bool(x)) { + rep_named(names, x) + } else if (vec_is_list(x)) { + check_unique_names(x, arg = arg, call = call) + x[intersect(names(x), names)] + } else { + cli::cli_abort( + "{.arg {arg}} must be a list or a single `TRUE` or `FALSE`.", + call = call + ) + } +} diff --git a/tests/testthat/_snaps/hoist.md b/tests/testthat/_snaps/hoist.md index 460e49e35..6058dd4a9 100644 --- a/tests/testthat/_snaps/hoist.md +++ b/tests/testthat/_snaps/hoist.md @@ -23,7 +23,7 @@ Output Error in `hoist()`: - ! `.col` must identify a list-column. + ! `.data[[.col]]` must be a list, not a number. Code (expect_error(df %>% hoist(x, 1))) Output @@ -44,14 +44,38 @@ Output Error in `hoist()`: - ! `.col` must identify a list-column. + ! `.data[[.col]]` must be a list, not a object. -# hoist() input must be a data frame (#1224) +# hoist() validates its inputs (#1224) Code - (expect_error(hoist(1))) - Output - + hoist(1) + Condition + Error in `hoist()`: + ! `.data` must be a data frame, not a string. + Code + hoist(df) + Condition + Error in `hoist()`: + ! `.col` is absent but must be supplied. + Code + hoist(df, a, .remove = 1) + Condition + Error in `hoist()`: + ! `.remove` must be `TRUE` or `FALSE`, not the number 1. + Code + hoist(df, a, .ptype = 1) + Condition + Error in `hoist()`: + ! `.ptype` must be `NULL`, an empty ptype, or a named list of ptypes. + Code + hoist(df, a, .transform = 1) + Condition + Error in `hoist()`: + ! `.transform` must be `NULL`, a function, or a named list of functions. + Code + hoist(df, a, .simplify = 1) + Condition Error in `hoist()`: - ! `.data` must be a data frame. + ! `.simplify` must be a list or a single `TRUE` or `FALSE`. diff --git a/tests/testthat/_snaps/pivot-long.md b/tests/testthat/_snaps/pivot-long.md index 725402df6..a3b2a7b59 100644 --- a/tests/testthat/_snaps/pivot-long.md +++ b/tests/testthat/_snaps/pivot-long.md @@ -125,7 +125,7 @@ Output Error in `build_longer_spec()`: - ! Can't convert `names_transform`, a number, to a function. + ! `names_transform` must be `NULL`, a function, or a named list of functions. Code (expect_error(build_longer_spec(df, x, names_transform = list(~.x)))) Output @@ -155,7 +155,7 @@ Output Error in `pivot_longer_spec()`: - ! Can't convert `values_transform`, a number, to a function. + ! `values_transform` must be `NULL`, a function, or a named list of functions. Code (expect_error(pivot_longer(df, x, values_transform = list(~.x)))) Output diff --git a/tests/testthat/_snaps/pivot-wide.md b/tests/testthat/_snaps/pivot-wide.md index f8fc15a8e..b42f86859 100644 --- a/tests/testthat/_snaps/pivot-wide.md +++ b/tests/testthat/_snaps/pivot-wide.md @@ -228,7 +228,7 @@ Output Error in `pivot_wider_spec()`: - ! Can't convert `values_fn`, a number, to a function. + ! `values_fn` must be `NULL`, a function, or a named list of functions. # `unused_fn` must result in single summary values @@ -247,5 +247,5 @@ Output Error in `pivot_wider_spec()`: - ! Can't convert `unused_fn`, a number, to a function. + ! `unused_fn` must be `NULL`, a function, or a named list of functions. diff --git a/tests/testthat/_snaps/unnest-helper.md b/tests/testthat/_snaps/unnest-helper.md index ba7273936..aa39b1b68 100644 --- a/tests/testthat/_snaps/unnest-helper.md +++ b/tests/testthat/_snaps/unnest-helper.md @@ -65,7 +65,7 @@ Output Error: - ! Can't convert `transform`, a number, to a function. + ! `transform` must be `NULL`, a function, or a named list of functions. Code (expect_error(df_simplify(data.frame(), transform = list(x = 1)))) Output diff --git a/tests/testthat/test-hoist.R b/tests/testthat/test-hoist.R index bd84963ba..5c36989ec 100644 --- a/tests/testthat/test-hoist.R +++ b/tests/testthat/test-hoist.R @@ -163,8 +163,17 @@ test_that("can hoist out a rcrd style column (#999)", { expect_identical(out$x, vec_c(x, x)) }) -test_that("hoist() input must be a data frame (#1224)", { - expect_snapshot((expect_error(hoist(1)))) +test_that("hoist() validates its inputs (#1224)", { + df <- tibble(a = list(1)) + + expect_snapshot(error = TRUE, { + hoist(1) + hoist(df) + hoist(df, a, .remove = 1) + hoist(df, a, .ptype = 1) + hoist(df, a, .transform = 1) + hoist(df, a, .simplify = 1) + }) }) test_that("hoist() can simplify on a per column basis (#995)", { From 8e2109a731e21f339b3fb5dafada1d901198087c Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 18 Oct 2022 11:35:14 -0500 Subject: [PATCH 08/26] Fix bug in check_data_frame() --- R/utils.R | 2 +- tests/testthat/_snaps/chop.md | 4 ++-- tests/testthat/_snaps/hoist.md | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/R/utils.R b/R/utils.R index 7f3a1f88a..3e2d6705e 100644 --- a/R/utils.R +++ b/R/utils.R @@ -195,7 +195,7 @@ vec_paste0 <- function(...) { check_data_frame <- function(x, arg = caller_arg(x), call = caller_env()) { if (!is.data.frame(x)) { - cli::cli_abort("{.arg {arg}} must be a data frame, not {.obj_type_friendly x}.", call = call) + cli::cli_abort("{.arg {arg}} must be a data frame, not {.obj_type_friendly {x}}.", call = call) } } diff --git a/tests/testthat/_snaps/chop.md b/tests/testthat/_snaps/chop.md index 21e30ea8a..e993908ab 100644 --- a/tests/testthat/_snaps/chop.md +++ b/tests/testthat/_snaps/chop.md @@ -4,7 +4,7 @@ chop(df$x) Condition Error in `chop()`: - ! `data` must be a data frame, not a string. + ! `data` must be a data frame, not an integer vector. Code chop(df) Condition @@ -35,7 +35,7 @@ unchop(1:10) Condition Error in `unchop()`: - ! `data` must be a data frame, not a string. + ! `data` must be a data frame, not an integer vector. Code unchop(df) Condition diff --git a/tests/testthat/_snaps/hoist.md b/tests/testthat/_snaps/hoist.md index 6058dd4a9..4bc7dbc5c 100644 --- a/tests/testthat/_snaps/hoist.md +++ b/tests/testthat/_snaps/hoist.md @@ -52,7 +52,7 @@ hoist(1) Condition Error in `hoist()`: - ! `.data` must be a data frame, not a string. + ! `.data` must be a data frame, not a number. Code hoist(df) Condition From 03fa1fd3135a33f118714e49f7989a02cf79404e Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 18 Oct 2022 11:43:15 -0500 Subject: [PATCH 09/26] Improve input validation for nest, pack, and unpack --- R/nest.R | 1 + R/pack.R | 11 ++++++---- tests/testthat/_snaps/nest.md | 8 +++++++ tests/testthat/_snaps/pack.md | 40 +++++++++++++++++++++++++++++------ tests/testthat/test-nest.R | 7 ++++++ tests/testthat/test-pack.R | 30 ++++++++++++++++++-------- 6 files changed, 77 insertions(+), 20 deletions(-) diff --git a/R/nest.R b/R/nest.R index d081f88fb..329eb7533 100644 --- a/R/nest.R +++ b/R/nest.R @@ -126,6 +126,7 @@ nest.data.frame <- function(.data, ..., .names_sep = NULL, .key = deprecated()) #' @export nest.tbl_df <- function(.data, ..., .names_sep = NULL, .key = deprecated()) { + check_string(.names_sep, allow_null = TRUE) .key <- check_key(.key) if (missing(...)) { warn(paste0( diff --git a/R/pack.R b/R/pack.R index b770cd730..0c79b01cf 100644 --- a/R/pack.R +++ b/R/pack.R @@ -58,10 +58,12 @@ #' df %>% unpack(c(y, z)) #' df %>% unpack(c(y, z), names_sep = "_") pack <- function(.data, ..., .names_sep = NULL) { + check_data_frame(.data) cols <- enquos(...) if (any(names2(cols) == "")) { - abort("All elements of `...` must be named") + cli::cli_abort("All elements of `...` must be named") } + check_string(.names_sep, allow_null = TRUE) cols <- map(cols, ~ tidyselect::eval_select(.x, .data)) @@ -97,14 +99,14 @@ pack <- function(.data, ..., .names_sep = NULL) { #' See [vctrs::vec_as_names()] for more details on these terms and the #' strategies used to enforce them. unpack <- function(data, cols, names_sep = NULL, names_repair = "check_unique") { + check_data_frame(data) check_required(cols) - cols <- tidyselect::eval_select(enquo(cols), data) - - size <- vec_size(data) + check_string(names_sep, allow_null = TRUE) # Start from first principles to avoid issues in any subclass methods out <- tidyr_new_list(data) + cols <- tidyselect::eval_select(enquo(cols), data) cols <- out[cols] cols <- cols[map_lgl(cols, is.data.frame)] @@ -124,6 +126,7 @@ unpack <- function(data, cols, names_sep = NULL, names_repair = "check_unique") names[names %in% cols_names] <- "" names(out) <- names + size <- vec_size(data) out <- df_list(!!!out, .size = size, .name_repair = "minimal") out <- tibble::new_tibble(out, nrow = size) diff --git a/tests/testthat/_snaps/nest.md b/tests/testthat/_snaps/nest.md index 51565430d..1acc6c548 100644 --- a/tests/testthat/_snaps/nest.md +++ b/tests/testthat/_snaps/nest.md @@ -7,6 +7,14 @@ `...` must not be empty for ungrouped data frames. Did you want `data = everything()`? +# validates its inputs + + Code + nest(df, y = ya:yb, .names_sep = 1) + Condition + Error in `nest()`: + ! `.names_sep` must be a single string or `NULL`, not the number 1. + # warn about old style interface Code diff --git a/tests/testthat/_snaps/pack.md b/tests/testthat/_snaps/pack.md index 6caad62c1..e6421f5f5 100644 --- a/tests/testthat/_snaps/pack.md +++ b/tests/testthat/_snaps/pack.md @@ -1,15 +1,41 @@ -# all inputs must be named +# pack validates its inputs Code - (expect_error(pack(df, a = c(a1, a2), c(b1, b2)))) - Output - + pack(1) + Condition + Error in `pack()`: + ! `.data` must be a data frame, not a number. + Code + pack(df, c(a1, a2), c(b1, b2)) + Condition Error in `pack()`: ! All elements of `...` must be named Code - (expect_error(pack(df, c(a1, a2), c(b1, b2)))) - Output - + pack(df, a = c(a1, a2), c(b1, b2)) + Condition Error in `pack()`: ! All elements of `...` must be named + Code + pack(df, a = c(a1, a2), .names_sep = 1) + Condition + Error in `pack()`: + ! `.names_sep` must be a single string or `NULL`, not the number 1. + +# unpack() validates its inputs + + Code + unpack(1) + Condition + Error in `unpack()`: + ! `data` must be a data frame, not a number. + Code + unpack(df) + Condition + Error in `unpack()`: + ! `cols` is absent but must be supplied. + Code + unpack(df, y, names_sep = 1) + Condition + Error in `unpack()`: + ! `names_sep` must be a single string or `NULL`, not the number 1. diff --git a/tests/testthat/test-nest.R b/tests/testthat/test-nest.R index abf97624c..4917a4be0 100644 --- a/tests/testthat/test-nest.R +++ b/tests/testthat/test-nest.R @@ -97,6 +97,13 @@ test_that("nesting no columns nests all inputs", { expect_equal(out$data[[1]], df) }) +test_that("validates its inputs", { + df <- tibble(x = c(1, 1, 1), ya = 1:3, yb = 4:6) + expect_snapshot(error = TRUE, { + nest(df, y = ya:yb, .names_sep = 1) + }) +}) + # Deprecated behaviours --------------------------------------------------- diff --git a/tests/testthat/test-pack.R b/tests/testthat/test-pack.R index 831022622..26f4a42e4 100644 --- a/tests/testthat/test-pack.R +++ b/tests/testthat/test-pack.R @@ -20,15 +20,6 @@ test_that("can strip outer names from inner names", { expect_named(out$a, c("x", "y")) }) -test_that("all inputs must be named", { - df <- tibble(a1 = 1, a2 = 2, b1 = 1, b2 = 2) - - expect_snapshot({ - (expect_error(pack(df, a = c(a1, a2), c(b1, b2)))) - (expect_error(pack(df, c(a1, a2), c(b1, b2)))) - }) -}) - test_that("grouping is preserved", { df <- tibble(g1 = 1, g2 = 1, g3 = 1) out <- df %>% @@ -37,6 +28,17 @@ test_that("grouping is preserved", { expect_equal(dplyr::group_vars(out), "g1") }) +test_that("pack validates its inputs", { + df <- tibble(a1 = 1, a2 = 2, b1 = 1, b2 = 2) + + expect_snapshot(error = TRUE,{ + pack(1) + pack(df, c(a1, a2), c(b1, b2)) + pack(df, a = c(a1, a2), c(b1, b2)) + pack(df, a = c(a1, a2), .names_sep = 1) + }) +}) + # unpack ------------------------------------------------------------------ test_that("grouping is preserved", { @@ -88,3 +90,13 @@ test_that("can unpack 1-row but 0-col dataframe (#1189)", { df <- tibble(x = tibble(.rows = 1)) expect_identical(unpack(df, x), tibble::new_tibble(list(), nrow = 1L)) }) + +test_that("unpack() validates its inputs", { + df <- tibble(x = 1:2, y = tibble(a = 1:2, b = 1:2)) + + expect_snapshot(error = TRUE, { + unpack(1) + unpack(df) + unpack(df, y, names_sep = 1) + }) +}) From 6f198212263922707143d857259d03dfd9e65332 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 18 Oct 2022 14:26:28 -0500 Subject: [PATCH 10/26] Use check helpers in check_pivot_spec And fix arg name --- R/pivot.R | 21 +++++++++------------ man/check_pivot_spec.Rd | 2 +- tests/testthat/_snaps/pivot.md | 8 ++++---- 3 files changed, 14 insertions(+), 17 deletions(-) diff --git a/R/pivot.R b/R/pivot.R index ed0e2fd84..a3d69b30b 100644 --- a/R/pivot.R +++ b/R/pivot.R @@ -29,25 +29,22 @@ #' # `.name` and `.value` are forced to be the first two columns #' spec <- tibble(foo = 1, .value = "b", .name = "a") #' check_pivot_spec(spec) -check_pivot_spec <- function(spec, error_call = caller_env()) { - if (!is.data.frame(spec)) { - abort("`spec` must be a data frame.", call = error_call) - } +check_pivot_spec <- function(spec, call = caller_env()) { + check_data_frame(spec, call = call) if (!has_name(spec, ".name") || !has_name(spec, ".value")) { - abort("`spec` must have `.name` and `.value` columns.", call = error_call) + cli::cli_abort( + "{.arg spec} must have {.var .name} and {.var .value} columns.", + call = call + ) } - if (!is.character(spec$.name)) { - abort("The `.name` column of `spec` must be a character vector.", call = error_call) - } + check_character(spec$.name, call = call) if (vec_duplicate_any(spec$.name)) { - abort("The `.name` column of `spec` must be unique.", call = error_call) + cli::cli_abort("{.var spec$.name} must be unique.", call = call) } - if (!is.character(spec$.value)) { - abort("The `.value` column of `spec` must be a character vector.", call = error_call) - } + check_character(spec$.value, call = call) # Ensure `.name` and `.value` come first, in that order vars <- union(c(".name", ".value"), names(spec)) diff --git a/man/check_pivot_spec.Rd b/man/check_pivot_spec.Rd index 9b14bee9f..4984d3f87 100644 --- a/man/check_pivot_spec.Rd +++ b/man/check_pivot_spec.Rd @@ -4,7 +4,7 @@ \alias{check_pivot_spec} \title{Check assumptions about a pivot \code{spec}} \usage{ -check_pivot_spec(spec, error_call = caller_env()) +check_pivot_spec(spec, call = caller_env()) } \arguments{ \item{spec}{A specification data frame. This is useful for more complex diff --git a/tests/testthat/_snaps/pivot.md b/tests/testthat/_snaps/pivot.md index a8ee673f7..86eb8a787 100644 --- a/tests/testthat/_snaps/pivot.md +++ b/tests/testthat/_snaps/pivot.md @@ -5,7 +5,7 @@ Output Error: - ! `spec` must be a data frame. + ! `spec` must be a data frame, not a number. Code (expect_error(check_pivot_spec(mtcars))) Output @@ -20,7 +20,7 @@ Output Error: - ! The `.name` column of `spec` must be a character vector. + ! `spec$.name` must be a character vector, not an integer vector. # `.value` column must be a character vector @@ -29,7 +29,7 @@ Output Error: - ! The `.value` column of `spec` must be a character vector. + ! `spec$.value` must be a character vector, not an integer vector. # `.name` column must be unique @@ -38,5 +38,5 @@ Output Error: - ! The `.name` column of `spec` must be unique. + ! `spec$.name` must be unique. From 42e176cfe1a08cee10154b5f5356b8f75253f593 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 18 Oct 2022 14:29:01 -0500 Subject: [PATCH 11/26] Polish replace_na argument validation --- R/replace_na.R | 2 +- tests/testthat/_snaps/replace_na.md | 8 ++++++++ tests/testthat/test-replace_na.R | 7 +++++++ 3 files changed, 16 insertions(+), 1 deletion(-) diff --git a/R/replace_na.R b/R/replace_na.R index 37e7504c9..c9076f9e1 100644 --- a/R/replace_na.R +++ b/R/replace_na.R @@ -44,7 +44,7 @@ replace_na.default <- function(data, replace = NA, ...) { #' @export replace_na.data.frame <- function(data, replace = list(), ...) { if (!vec_is_list(replace)) { - abort("`replace` must be a list.") + cli::cli_abort("{.arg replace} must be a list, not {.obj_type_friendly {replace}}.") } names <- intersect(names(replace), names(data)) diff --git a/tests/testthat/_snaps/replace_na.md b/tests/testthat/_snaps/replace_na.md index 364412150..6d784fa3d 100644 --- a/tests/testthat/_snaps/replace_na.md +++ b/tests/testthat/_snaps/replace_na.md @@ -27,3 +27,11 @@ ! Can't convert from `replace$a` to `data$a` due to loss of precision. * Locations: 1 +# validates its inputs + + Code + replace_na(df, replace = 1) + Condition + Error in `replace_na()`: + ! `replace` must be a list, not a number. + diff --git a/tests/testthat/test-replace_na.R b/tests/testthat/test-replace_na.R index 5d029eace..9254f41ac 100644 --- a/tests/testthat/test-replace_na.R +++ b/tests/testthat/test-replace_na.R @@ -97,3 +97,10 @@ test_that("replacement must be castable to corresponding column", { df <- tibble(a = c(1L, NA)) expect_snapshot((expect_error(replace_na(df, list(a = 1.5))))) }) + +test_that("validates its inputs", { + df <- tibble(a = c(1L, NA)) + expect_snapshot(error = TRUE, { + replace_na(df, replace = 1) + }) +}) From cffa70dbb246746d6bbb7315b2e325fcc84d77b5 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 18 Oct 2022 14:30:23 -0500 Subject: [PATCH 12/26] Improve separate_rows input validation --- R/separate-rows.R | 3 +++ tests/testthat/_snaps/separate-rows.md | 13 +++++++++++++ tests/testthat/test-separate-rows.R | 9 +++++++++ 3 files changed, 25 insertions(+) create mode 100644 tests/testthat/_snaps/separate-rows.md diff --git a/R/separate-rows.R b/R/separate-rows.R index 6a1b891d2..52577b883 100644 --- a/R/separate-rows.R +++ b/R/separate-rows.R @@ -29,6 +29,9 @@ separate_rows.data.frame <- function(data, ..., sep = "[^[:alnum:].]+", convert = FALSE) { + check_string(sep) + check_bool(convert) + vars <- tidyselect::eval_select(expr(c(...)), data, allow_rename = FALSE) out <- purrr::modify_at(data, vars, str_split_n, pattern = sep) diff --git a/tests/testthat/_snaps/separate-rows.md b/tests/testthat/_snaps/separate-rows.md new file mode 100644 index 000000000..568a67680 --- /dev/null +++ b/tests/testthat/_snaps/separate-rows.md @@ -0,0 +1,13 @@ +# it validates its inputs + + Code + separate_rows(df, x, sep = 1) + Condition + Error in `separate_rows()`: + ! `sep` must be a single string, not the number 1. + Code + separate_rows(df, x, convert = 1) + Condition + Error in `separate_rows()`: + ! `convert` must be `TRUE` or `FALSE`, not the number 1. + diff --git a/tests/testthat/test-separate-rows.R b/tests/testthat/test-separate-rows.R index 6b6fe80f9..8a096fbca 100644 --- a/tests/testthat/test-separate-rows.R +++ b/tests/testthat/test-separate-rows.R @@ -69,3 +69,12 @@ test_that("does not silently drop blank values (#1014)", { tibble(x = c(1, 2, 2, 2, 3), y = c("a", "d", "e", "f", "")) ) }) + +test_that("it validates its inputs", { + df <- tibble(x = 1:3, y = c("a", "d,e,f", "")) + + expect_snapshot(error = TRUE, { + separate_rows(df, x, sep = 1) + separate_rows(df, x, convert = 1) + }) +}) From ed74466ad8bc5df511e816ceebd3799525acf229 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 18 Oct 2022 15:12:01 -0500 Subject: [PATCH 13/26] Improve separate() input validation --- R/separate.R | 17 +++++++++------- tests/testthat/_snaps/separate.md | 33 +++++++++++++++++++++---------- tests/testthat/test-separate.R | 13 +++++++----- 3 files changed, 41 insertions(+), 22 deletions(-) diff --git a/R/separate.R b/R/separate.R index c07cd40bf..3b52cfcb1 100644 --- a/R/separate.R +++ b/R/separate.R @@ -72,6 +72,8 @@ separate.data.frame <- function(data, col, into, sep = "[^[:alnum:]]+", remove = TRUE, convert = FALSE, extra = "warn", fill = "warn", ...) { check_required(col) + check_bool(remove) + var <- tidyselect::vars_pull(names(data), !!enquo(col)) value <- as.character(data[[var]]) @@ -87,18 +89,19 @@ separate.data.frame <- function(data, col, into, sep = "[^[:alnum:]]+", } str_separate <- function(x, into, sep, convert = FALSE, extra = "warn", fill = "warn", error_call = caller_env()) { - check_not_stringr_pattern(sep, call = error_call) - - if (!is.character(into)) { - abort("`into` must be a character vector.", call = error_call) - } + check_character(into, call = error_call) + check_bool(convert, call = error_call) if (is.numeric(sep)) { out <- strsep(x, sep) - } else if (is_character(sep)) { + } else if (is_string(sep)) { + check_not_stringr_pattern(sep, call = error_call) out <- str_split_fixed(x, sep, length(into), extra = extra, fill = fill) } else { - abort("`sep` must be either numeric or character.", call = error_call) + cli::cli_abort( + "{.arg sep} must be a string or numeric vector, not {.obj_type_friendly {sep}}", + call = error_call + ) } names(out) <- as_utf8_character(into) diff --git a/tests/testthat/_snaps/separate.md b/tests/testthat/_snaps/separate.md index 701e4615f..33c9e861e 100644 --- a/tests/testthat/_snaps/separate.md +++ b/tests/testthat/_snaps/separate.md @@ -42,25 +42,38 @@ 1 a b 2 a b c -# checks type of `into` and `sep` +# validates inputs Code - (expect_error(separate(df, x, "x", FALSE))) - Output - + separate(df) + Condition Error in `separate()`: - ! `sep` must be either numeric or character. + ! `col` is absent but must be supplied. Code - (expect_error(separate(df, x, FALSE))) - Output - + separate(df, x, into = 1) + Condition + Error in `separate()`: + ! `into` must be a character vector, not the number 1. + Code + separate(df, x, into = "x", sep = c("a", "b")) + Condition + Error in `separate()`: + ! `sep` must be a string or numeric vector, not a character vector + Code + separate(df, x, into = "x", remove = 1) + Condition + Error in `separate()`: + ! `remove` must be `TRUE` or `FALSE`, not the number 1. + Code + separate(df, x, into = "x", convert = 1) + Condition Error in `separate()`: - ! `into` must be a character vector. + ! `convert` must be `TRUE` or `FALSE`, not the number 1. # informative error if using stringr modifier functions (#693) Code - (expect_error(separate(df, x, sep = sep))) + (expect_error(separate(df, x, "x", sep = sep))) Output Error in `separate()`: diff --git a/tests/testthat/test-separate.R b/tests/testthat/test-separate.R index 5d602407e..660a7d2c3 100644 --- a/tests/testthat/test-separate.R +++ b/tests/testthat/test-separate.R @@ -105,12 +105,15 @@ test_that("drops NA columns", { expect_equal(out$y, c(NA, "b", "d")) }) -test_that("checks type of `into` and `sep`", { +test_that("validates inputs", { df <- tibble(x = "a:b") - expect_snapshot({ - (expect_error(separate(df, x, "x", FALSE))) - (expect_error(separate(df, x, FALSE))) + expect_snapshot(error = TRUE, { + separate(df) + separate(df, x, into = 1) + separate(df, x, into = "x", sep = c("a", "b")) + separate(df, x, into = "x", remove = 1) + separate(df, x, into = "x", convert = 1) }) }) @@ -118,7 +121,7 @@ test_that("informative error if using stringr modifier functions (#693)", { df <- tibble(x = "a") sep <- structure("a", class = "pattern") - expect_snapshot((expect_error(separate(df, x, sep = sep)))) + expect_snapshot((expect_error(separate(df, x, "x", sep = sep)))) }) # helpers ----------------------------------------------------------------- From e22ac9dacc4466a48311bd41cc31aea8dc715c70 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 18 Oct 2022 15:15:00 -0500 Subject: [PATCH 14/26] Add input validation to full_seq() And update error --- R/seq.R | 6 ++++-- tests/testthat/_snaps/seq.md | 21 +++++++++++++++++++++ tests/testthat/test-seq.R | 14 ++++++++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) create mode 100644 tests/testthat/_snaps/seq.md create mode 100644 tests/testthat/test-seq.R diff --git a/R/seq.R b/R/seq.R index f10d02a78..8761dc1fb 100644 --- a/R/seq.R +++ b/R/seq.R @@ -17,10 +17,13 @@ full_seq <- function(x, period, tol = 1e-6) { #' @export full_seq.numeric <- function(x, period, tol = 1e-6) { + check_number_decimal(period) + check_number_decimal(tol, min = 0) + rng <- range(x, na.rm = TRUE) if (any(((x - rng[1]) %% period > tol) & (period - (x - rng[1]) %% period > tol))) { - stop("`x` is not a regular sequence.", call. = FALSE) + cli::cli_abort("{.arg x} is not a regular sequence.") } # in cases where the last element is within tolerance, pad it so that @@ -42,7 +45,6 @@ full_seq.POSIXct <- function(x, period, tol = 1e-6) { restore(x, full_seq(as.numeric(x), period, tol)) } - restore <- function(old, new) { mostattributes(new) <- attributes(old) new diff --git a/tests/testthat/_snaps/seq.md b/tests/testthat/_snaps/seq.md new file mode 100644 index 000000000..b0651d295 --- /dev/null +++ b/tests/testthat/_snaps/seq.md @@ -0,0 +1,21 @@ +# errors if sequence not regular + + Code + full_seq(x, 1) + Condition + Error in `full_seq()`: + ! `x` is not a regular sequence. + +# validates inputs + + Code + full_seq(x, period = "a") + Condition + Error in `full_seq()`: + ! `period` must be a number, not the string "a". + Code + full_seq(x, 1, tol = "a") + Condition + Error in `full_seq()`: + ! `tol` must be a number, not the string "a". + diff --git a/tests/testthat/test-seq.R b/tests/testthat/test-seq.R new file mode 100644 index 000000000..d82e16b20 --- /dev/null +++ b/tests/testthat/test-seq.R @@ -0,0 +1,14 @@ +test_that("errors if sequence not regular", { + x <- c(1, 2, 2.5, 4) + expect_snapshot(error = TRUE, { + full_seq(x, 1) + }) +}) + +test_that("validates inputs", { + x <- 1:5 + expect_snapshot(error = TRUE, { + full_seq(x, period = "a") + full_seq(x, 1, tol = "a") + }) +}) From 58e0bf9683510481d20eafa2d26dfd41a1f7562b Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 18 Oct 2022 15:19:09 -0500 Subject: [PATCH 15/26] Improve uncount() input validation --- R/uncount.R | 13 ++++--------- tests/testthat/_snaps/uncount.md | 24 +++++++++++++++++++----- tests/testthat/test-uncount.R | 12 +++++++++--- 3 files changed, 32 insertions(+), 17 deletions(-) diff --git a/R/uncount.R b/R/uncount.R index ce4be985c..36b79973f 100644 --- a/R/uncount.R +++ b/R/uncount.R @@ -29,18 +29,13 @@ uncount <- function(data, weights, ..., .remove = TRUE, .id = NULL) { #' @export uncount.data.frame <- function(data, weights, ..., .remove = TRUE, .id = NULL) { + check_bool(.remove) + check_name(.id, allow_null = TRUE) + weights_quo <- enquo(weights) w <- dplyr::pull(dplyr::mutate(data, `_weight` = !!weights_quo)) - # NOTE `vec_cast()` and check for positive weights can be removed - # if `vec_rep_each()` gets a `x_arg` argument - # https://github.com/r-lib/vctrs/issues/1303 - w <- vec_cast(w, integer(), x_arg = "weights") - - if (any(w < 0)) { - abort("all elements of `weights` must be >= 0") - } - out <- vec_rep_each(data, w) + out <- vec_rep_each(data, w, error_call = current_env(), times_arg = "weights") # NOTE it was decided to also remove grouping variables as there is no clear # best answer. See https://github.com/tidyverse/tidyr/pull/1070 diff --git a/tests/testthat/_snaps/uncount.md b/tests/testthat/_snaps/uncount.md index 235e7c507..23a595de6 100644 --- a/tests/testthat/_snaps/uncount.md +++ b/tests/testthat/_snaps/uncount.md @@ -1,9 +1,23 @@ -# errors on negative weights +# validates inputs Code - (expect_error(uncount(df, w))) - Output - + uncount(df, y) + Condition Error in `uncount()`: - ! all elements of `weights` must be >= 0 + ! Can't convert `weights` to . + Code + uncount(df, w) + Condition + Error in `uncount()`: + ! `weights` must be a vector of positive numbers. Location 1 is negative. + Code + uncount(df, x, .remove = 1) + Condition + Error in `uncount()`: + ! `.remove` must be `TRUE` or `FALSE`, not the number 1. + Code + uncount(df, x, .id = "") + Condition + Error in `uncount()`: + ! `.id` must be a valid name or `NULL`, not the empty string "". diff --git a/tests/testthat/test-uncount.R b/tests/testthat/test-uncount.R index d82923b2e..66e622623 100644 --- a/tests/testthat/test-uncount.R +++ b/tests/testthat/test-uncount.R @@ -39,7 +39,13 @@ test_that("works with 0 weights", { expect_equal(uncount(df, w), tibble(x = 2)) }) -test_that("errors on negative weights", { - df <- tibble(x = 1, w = -1) - expect_snapshot((expect_error(uncount(df, w)))) +test_that("validates inputs", { + df <- tibble(x = 1, y = "a", w = -1) + + expect_snapshot(error = TRUE, { + uncount(df, y) + uncount(df, w) + uncount(df, x, .remove = 1) + uncount(df, x, .id = "") + }) }) From 937ff717f3766be227fed22abe08210073feea58 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 18 Oct 2022 15:20:26 -0500 Subject: [PATCH 16/26] Combine full_seq() tests --- tests/testthat/_snaps/full_seq.md | 8 ++++++-- tests/testthat/_snaps/seq.md | 13 ++++++++++--- tests/testthat/test-full_seq.R | 30 ----------------------------- tests/testthat/test-seq.R | 32 +++++++++++++++++++++++++++---- 4 files changed, 44 insertions(+), 39 deletions(-) delete mode 100644 tests/testthat/test-full_seq.R diff --git a/tests/testthat/_snaps/full_seq.md b/tests/testthat/_snaps/full_seq.md index fc660211e..8d760331a 100644 --- a/tests/testthat/_snaps/full_seq.md +++ b/tests/testthat/_snaps/full_seq.md @@ -3,9 +3,13 @@ Code (expect_error(full_seq(c(1, 3, 4), 2))) Output - + + Error in `full_seq()`: + ! `x` is not a regular sequence. Code (expect_error(full_seq(c(0, 10, 20), 11, tol = 1.8))) Output - + + Error in `full_seq()`: + ! `x` is not a regular sequence. diff --git a/tests/testthat/_snaps/seq.md b/tests/testthat/_snaps/seq.md index b0651d295..954eb6d91 100644 --- a/tests/testthat/_snaps/seq.md +++ b/tests/testthat/_snaps/seq.md @@ -1,8 +1,15 @@ -# errors if sequence not regular +# full_seq errors if sequence isn't regular Code - full_seq(x, 1) - Condition + (expect_error(full_seq(c(1, 3, 4), 2))) + Output + + Error in `full_seq()`: + ! `x` is not a regular sequence. + Code + (expect_error(full_seq(c(0, 10, 20), 11, tol = 1.8))) + Output + Error in `full_seq()`: ! `x` is not a regular sequence. diff --git a/tests/testthat/test-full_seq.R b/tests/testthat/test-full_seq.R deleted file mode 100644 index 357d961ed..000000000 --- a/tests/testthat/test-full_seq.R +++ /dev/null @@ -1,30 +0,0 @@ -test_that("full_seq errors if sequence isn't regular", { - expect_snapshot({ - (expect_error(full_seq(c(1, 3, 4), 2))) - (expect_error(full_seq(c(0, 10, 20), 11, tol = 1.8))) - }) -}) - -test_that("full_seq with tol > 0 allows sequences to fall short of period", { - expect_equal(full_seq(c(0, 10, 20), 11, tol = 2), c(0, 11, 22)) -}) - -test_that("full_seq pads length correctly for tol > 0", { - expect_equal(full_seq(c(0, 10, 16), 11, tol = 5), c(0, 11)) -}) - -test_that("sequences don't have to start at zero", { - expect_equal(full_seq(c(1, 5), 2), c(1, 3, 5)) -}) - -test_that("full_seq fills in gaps", { - expect_equal(full_seq(c(1, 3), 1), c(1, 2, 3)) -}) - -test_that("preserves attributes", { - x1 <- as.Date("2001-01-01") + c(0, 2) - x2 <- as.POSIXct(x1) - - expect_s3_class(full_seq(x1, 2), "Date") - expect_s3_class(full_seq(x2, 86400), c("POSIXct", "POSIXt")) -}) diff --git a/tests/testthat/test-seq.R b/tests/testthat/test-seq.R index d82e16b20..10e0f0123 100644 --- a/tests/testthat/test-seq.R +++ b/tests/testthat/test-seq.R @@ -1,7 +1,31 @@ -test_that("errors if sequence not regular", { - x <- c(1, 2, 2.5, 4) - expect_snapshot(error = TRUE, { - full_seq(x, 1) +test_that("full_seq with tol > 0 allows sequences to fall short of period", { + expect_equal(full_seq(c(0, 10, 20), 11, tol = 2), c(0, 11, 22)) +}) + +test_that("full_seq pads length correctly for tol > 0", { + expect_equal(full_seq(c(0, 10, 16), 11, tol = 5), c(0, 11)) +}) + +test_that("sequences don't have to start at zero", { + expect_equal(full_seq(c(1, 5), 2), c(1, 3, 5)) +}) + +test_that("full_seq fills in gaps", { + expect_equal(full_seq(c(1, 3), 1), c(1, 2, 3)) +}) + +test_that("preserves attributes", { + x1 <- as.Date("2001-01-01") + c(0, 2) + x2 <- as.POSIXct(x1) + + expect_s3_class(full_seq(x1, 2), "Date") + expect_s3_class(full_seq(x2, 86400), c("POSIXct", "POSIXt")) +}) + +test_that("full_seq errors if sequence isn't regular", { + expect_snapshot({ + (expect_error(full_seq(c(1, 3, 4), 2))) + (expect_error(full_seq(c(0, 10, 20), 11, tol = 1.8))) }) }) From b26fd2ac0b557ff02ea9afa9ed721fefbb6e29d3 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 18 Oct 2022 15:22:40 -0500 Subject: [PATCH 17/26] Improve unite() input validation --- R/unite.R | 5 +++++ tests/testthat/_snaps/full_seq.md | 15 --------------- tests/testthat/_snaps/unite.md | 23 +++++++++++++++++++++++ tests/testthat/test-unite.R | 11 +++++++++++ 4 files changed, 39 insertions(+), 15 deletions(-) delete mode 100644 tests/testthat/_snaps/full_seq.md create mode 100644 tests/testthat/_snaps/unite.md diff --git a/R/unite.R b/R/unite.R index a4b0fe71d..d9a0e3200 100644 --- a/R/unite.R +++ b/R/unite.R @@ -37,6 +37,11 @@ unite <- function(data, col, ..., sep = "_", remove = TRUE, na.rm = FALSE) { } #' @export unite.data.frame <- function(data, col, ..., sep = "_", remove = TRUE, na.rm = FALSE) { + check_required(col) + check_string(sep) + check_bool(remove) + check_bool(na.rm) + if (dots_n(...) == 0) { from_vars <- set_names(seq_along(data), names(data)) } else { diff --git a/tests/testthat/_snaps/full_seq.md b/tests/testthat/_snaps/full_seq.md deleted file mode 100644 index 8d760331a..000000000 --- a/tests/testthat/_snaps/full_seq.md +++ /dev/null @@ -1,15 +0,0 @@ -# full_seq errors if sequence isn't regular - - Code - (expect_error(full_seq(c(1, 3, 4), 2))) - Output - - Error in `full_seq()`: - ! `x` is not a regular sequence. - Code - (expect_error(full_seq(c(0, 10, 20), 11, tol = 1.8))) - Output - - Error in `full_seq()`: - ! `x` is not a regular sequence. - diff --git a/tests/testthat/_snaps/unite.md b/tests/testthat/_snaps/unite.md new file mode 100644 index 000000000..ae9630cd3 --- /dev/null +++ b/tests/testthat/_snaps/unite.md @@ -0,0 +1,23 @@ +# validates its inputs + + Code + unite(df) + Condition + Error in `unite()`: + ! `col` is absent but must be supplied. + Code + unite(df, "z", x:y, sep = 1) + Condition + Error in `unite()`: + ! `sep` must be a single string, not the number 1. + Code + unite(df, "z", x:y, remove = 1) + Condition + Error in `unite()`: + ! `remove` must be `TRUE` or `FALSE`, not the number 1. + Code + unite(df, "z", x:y, na.rm = 1) + Condition + Error in `unite()`: + ! `na.rm` must be `TRUE` or `FALSE`, not the number 1. + diff --git a/tests/testthat/test-unite.R b/tests/testthat/test-unite.R index 215038d0d..17a55e47b 100644 --- a/tests/testthat/test-unite.R +++ b/tests/testthat/test-unite.R @@ -55,3 +55,14 @@ test_that("regardless of the type of the NA", { expect_equal(vec_unite(df, c("x", "dbl")), c("x", "y", "z")) expect_equal(vec_unite(df, c("x", "chr")), c("x", "y", "z")) }) + +test_that("validates its inputs", { + df <- tibble(x = "a", y = "b") + + expect_snapshot(error = TRUE, { + unite(df) + unite(df, "z", x:y, sep = 1) + unite(df, "z", x:y, remove = 1) + unite(df, "z", x:y, na.rm = 1) + }) +}) From 665a593eed8e6acb78672de9c7632da14a61ad6f Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 18 Oct 2022 15:50:38 -0500 Subject: [PATCH 18/26] Improve input validation for unnest_longer() --- R/unnest-longer.R | 34 ++++++++------------- tests/testthat/_snaps/unnest-longer.md | 41 +++++++++++++++++++------- tests/testthat/test-unnest-longer.R | 11 +++++-- 3 files changed, 52 insertions(+), 34 deletions(-) diff --git a/R/unnest-longer.R b/R/unnest-longer.R index edc03e604..715faa8f8 100644 --- a/R/unnest-longer.R +++ b/R/unnest-longer.R @@ -73,37 +73,29 @@ unnest_longer <- function(data, simplify = TRUE, ptype = NULL, transform = NULL) { - if (!is.data.frame(data)) { - abort("`data` must be a data frame.") - } + check_data_frame(data) check_required(col) + check_name(values_to, allow_null = TRUE) + check_name(indices_to, allow_null = TRUE) + check_bool(indices_include, allow_null = TRUE) + cols <- tidyselect::eval_select(enquo(col), data, allow_rename = FALSE) col_names <- names(cols) n_col_names <- length(col_names) - if (!is.null(indices_include) && !is_bool(indices_include)) { - abort("`indices_include` must be `NULL` or a single `TRUE` or `FALSE`.") - } - - if (is.null(values_to)) { - values_to <- "{col}" - } - if (!is_string(values_to)) { - abort("`values_to` must be a single string or `NULL`.") - } + values_to <- values_to %||% "{col}" if (is.null(indices_to)) { indices_to <- vec_paste0(values_to, "_id") } else { if (is_false(indices_include)) { - abort("Can't set `indices_include` to `FALSE` when `indices_to` is supplied.") + cli::cli_abort( + "Can't set {.arg indices_include} to {.code FALSE} when {.arg indices_to} is supplied." + ) } indices_include <- TRUE } - if (!is_string(indices_to)) { - abort("`indices_to` must be a single string or `NULL`.") - } values_to <- glue_col_names(values_to, col_names) values_to <- vec_recycle(values_to, size = n_col_names) @@ -149,9 +141,6 @@ col_to_long <- function(col, indices_to, indices_include, error_call = caller_env()) { - if (is.null(col)) { - abort(glue("Invalid `NULL` column detected for column `{name}`."), call = error_call) - } if (!vec_is_list(col)) { ptype <- vec_ptype(col) @@ -224,7 +213,10 @@ elt_to_long <- function(x, } if (!vec_is(x)) { - abort(glue("Column `{name}` must contain a list of vectors."), call = error_call) + cli::cli_abort( + "Column {.var {name}} must contain a list of vectors.", + call = error_call + ) } if (indices_include) { diff --git a/tests/testthat/_snaps/unnest-longer.md b/tests/testthat/_snaps/unnest-longer.md index 85a0e879c..2cb4027c4 100644 --- a/tests/testthat/_snaps/unnest-longer.md +++ b/tests/testthat/_snaps/unnest-longer.md @@ -17,14 +17,33 @@ Error in `unnest_longer()`: ! Can't set `indices_include` to `FALSE` when `indices_to` is supplied. -# unnest_longer() input must be a data frame (#1224) +# unnest_longer() validates its inputs Code - (expect_error(unnest_longer(1))) - Output - + unnest_longer(1) + Condition + Error in `unnest_longer()`: + ! `data` must be a data frame, not a number. + Code + unnest_longer(df) + Condition + Error in `unnest_longer()`: + ! `col` is absent but must be supplied. + Code + unnest_longer(df, x, indices_to = "") + Condition + Error in `unnest_longer()`: + ! `indices_to` must be a valid name or `NULL`, not the empty string "". + Code + unnest_longer(df, x, indices_include = 1) + Condition + Error in `unnest_longer()`: + ! `indices_include` must be `TRUE`, `FALSE`, or `NULL`, not the number 1. + Code + unnest_longer(df, x, values_to = "") + Condition Error in `unnest_longer()`: - ! `data` must be a data frame. + ! `values_to` must be a valid name or `NULL`, not the empty string "". # `values_to` is validated @@ -33,13 +52,13 @@ Output Error in `unnest_longer()`: - ! `values_to` must be a single string or `NULL`. + ! `values_to` must be a valid name or `NULL`, not the number 1. Code (expect_error(unnest_longer(mtcars, mpg, values_to = c("x", "y")))) Output Error in `unnest_longer()`: - ! `values_to` must be a single string or `NULL`. + ! `values_to` must be a valid name or `NULL`, not a character vector. # `indices_to` is validated @@ -48,13 +67,13 @@ Output Error in `unnest_longer()`: - ! `indices_to` must be a single string or `NULL`. + ! `indices_to` must be a valid name or `NULL`, not the number 1. Code (expect_error(unnest_longer(mtcars, mpg, indices_to = c("x", "y")))) Output Error in `unnest_longer()`: - ! `indices_to` must be a single string or `NULL`. + ! `indices_to` must be a valid name or `NULL`, not a character vector. # `indices_include` is validated @@ -63,11 +82,11 @@ Output Error in `unnest_longer()`: - ! `indices_include` must be `NULL` or a single `TRUE` or `FALSE`. + ! `indices_include` must be `TRUE`, `FALSE`, or `NULL`, not the number 1. Code (expect_error(unnest_longer(mtcars, mpg, indices_include = c(TRUE, FALSE)))) Output Error in `unnest_longer()`: - ! `indices_include` must be `NULL` or a single `TRUE` or `FALSE`. + ! `indices_include` must be `TRUE`, `FALSE`, or `NULL`, not a logical vector. diff --git a/tests/testthat/test-unnest-longer.R b/tests/testthat/test-unnest-longer.R index af909519f..62efa405b 100644 --- a/tests/testthat/test-unnest-longer.R +++ b/tests/testthat/test-unnest-longer.R @@ -265,8 +265,15 @@ test_that("can't mix `indices_to` with `indices_include = FALSE`", { ))) }) -test_that("unnest_longer() input must be a data frame (#1224)", { - expect_snapshot((expect_error(unnest_longer(1)))) +test_that("unnest_longer() validates its inputs", { + df <- tibble(x = list(list(a = 1L), list(b = 1L))) + expect_snapshot(error = TRUE, { + unnest_longer(1) + unnest_longer(df) + unnest_longer(df, x, indices_to = "") + unnest_longer(df, x, indices_include = 1) + unnest_longer(df, x, values_to = "") + }) }) test_that("`values_to` and `indices_to` glue can't reach into surrounding env", { From f214d61c19278928b2d69347e2ffb48aa8a172e4 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 18 Oct 2022 15:55:15 -0500 Subject: [PATCH 19/26] Improve unnest_wider() input validation --- R/unnest-wider.R | 18 ++++-------------- tests/testthat/_snaps/unnest-wider.md | 24 +++++++++++++++++++----- tests/testthat/test-unnest-wider.R | 10 ++++++++-- 3 files changed, 31 insertions(+), 21 deletions(-) diff --git a/R/unnest-wider.R b/R/unnest-wider.R index 715ae3148..663fcd113 100644 --- a/R/unnest-wider.R +++ b/R/unnest-wider.R @@ -83,21 +83,15 @@ unnest_wider <- function(data, names_repair = "check_unique", ptype = NULL, transform = NULL) { - if (!is.data.frame(data)) { - abort("`data` must be a data frame.") - } + check_data_frame(data) check_required(col) + check_string(names_sep, allow_null = TRUE) + check_bool(strict) + cols <- tidyselect::eval_select(enquo(col), data, allow_rename = FALSE) col_names <- names(cols) - if (!is.null(names_sep) && !is_string(names_sep)) { - abort("`names_sep` must be a single string or `NULL`.") - } - if (!is_bool(strict)) { - abort("`strict` must be a single `TRUE` or `FALSE`.") - } - for (i in seq_along(cols)) { col <- cols[[i]] col_name <- col_names[[i]] @@ -128,10 +122,6 @@ unnest_wider <- function(data, # Converts a column of any type to a `list_of` col_to_wide <- function(col, name, strict, names_sep, error_call = caller_env()) { - if (is.null(col)) { - abort(glue("Invalid `NULL` column detected for column `{name}`."), call = error_call) - } - if (!vec_is_list(col)) { ptype <- vec_ptype(col) col <- vec_chop(col) diff --git a/tests/testthat/_snaps/unnest-wider.md b/tests/testthat/_snaps/unnest-wider.md index 384fc6259..60b0217a8 100644 --- a/tests/testthat/_snaps/unnest-wider.md +++ b/tests/testthat/_snaps/unnest-wider.md @@ -52,12 +52,26 @@ New names: * `` -> `...1` -# unnest_wider() input must be a data frame (#1224) +# unnest_wider() validates its inputs Code - (expect_error(unnest_wider(1))) - Output - + unnest_wider(1) + Condition + Error in `unnest_wider()`: + ! `data` must be a data frame, not a number. + Code + unnest_wider(df) + Condition + Error in `unnest_wider()`: + ! `col` is absent but must be supplied. + Code + unnest_wider(df, x, names_sep = 1) + Condition + Error in `unnest_wider()`: + ! `names_sep` must be a single string or `NULL`, not the number 1. + Code + unnest_wider(df, x, strict = 1) + Condition Error in `unnest_wider()`: - ! `data` must be a data frame. + ! `strict` must be `TRUE` or `FALSE`, not the number 1. diff --git a/tests/testthat/test-unnest-wider.R b/tests/testthat/test-unnest-wider.R index 6b6ca7364..11afe8bc4 100644 --- a/tests/testthat/test-unnest-wider.R +++ b/tests/testthat/test-unnest-wider.R @@ -232,8 +232,14 @@ test_that("can combine ` + >`", { expect_identical(out$a, list(1:2, 1L)) }) -test_that("unnest_wider() input must be a data frame (#1224)", { - expect_snapshot((expect_error(unnest_wider(1)))) +test_that("unnest_wider() validates its inputs", { + df <- tibble(x = list(a = 1:2, b = 3:4)) + expect_snapshot(error = TRUE, { + unnest_wider(1) + unnest_wider(df) + unnest_wider(df, x, names_sep = 1) + unnest_wider(df, x, strict = 1) + }) }) test_that("invariant - final number of columns depends on element sizes", { From d489200efe63885beacd2c58513b71d856bdbca8 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 18 Oct 2022 16:00:51 -0500 Subject: [PATCH 20/26] Add cli --- DESCRIPTION | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index 501c8350c..e2a74a961 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -21,6 +21,7 @@ BugReports: https://github.com/tidyverse/tidyr/issues Depends: R (>= 3.1) Imports: + cli (>= 3.4.1), dplyr (>= 1.0.10), glue, lifecycle (>= 1.0.3), @@ -51,5 +52,5 @@ LazyData: true Roxygen: list(markdown = TRUE) RoxygenNote: 7.2.1 SystemRequirements: C++11 -Remotes: +Remotes: r-lib/vctrs From b5646caf20373eb9063cd5626f6b394ccc1ae74f Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 18 Oct 2022 16:04:07 -0500 Subject: [PATCH 21/26] Polish chop errors --- R/chop.R | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/R/chop.R b/R/chop.R index c6d6fbe17..ce7d6c97a 100644 --- a/R/chop.R +++ b/R/chop.R @@ -148,7 +148,7 @@ unchop <- function(data, cols, keep_empty = FALSE, ptype = NULL) { df_unchop <- function(x, ..., ptype = NULL, keep_empty = FALSE, error_call = caller_env()) { check_dots_empty() - ptype <- check_list_of_ptypes(ptype, names = names(x), arg = "ptype", call = error_call) + ptype <- check_list_of_ptypes(ptype, names = names(x), call = error_call) size <- vec_size(x) @@ -243,7 +243,7 @@ df_unchop <- function(x, ..., ptype = NULL, keep_empty = FALSE, error_call = cal # - `col` was an empty list(), or a list of all `NULL`s. # - No ptype was specified for `col`, either by the user or by a list-of. if (out_size != 0L) { - abort("Internal error: `NULL` column generated, but output size is not `0`.") + abort("`NULL` column generated, but output size is not `0`.", .internal = TRUE) } col <- unspecified(0L) @@ -283,7 +283,10 @@ unchop_sizes2 <- function(x, y, error_call) { row <- which(incompatible)[[1]] x <- x[[row]] y <- y[[row]] - abort(glue("In row {row}, can't recycle input of size {x} to size {y}."), call = error_call) + cli::cli_abort( + "In row {row}, can't recycle input of size {x} to size {y}.", + call = error_call + ) } x From 8239b37c18340c5ea36a4613953a71e2ea913dd7 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 18 Oct 2022 16:04:37 -0500 Subject: [PATCH 22/26] Polish complete errors --- R/complete.R | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/R/complete.R b/R/complete.R index 36c2a2e16..e6d704391 100644 --- a/R/complete.R +++ b/R/complete.R @@ -65,8 +65,6 @@ complete <- function(data, fill = list(), explicit = TRUE) { - check_bool(explicit) - UseMethod("complete") } @@ -79,6 +77,8 @@ complete.data.frame <- function(data, ..., fill = list(), explicit = TRUE) { + check_bool(explicit) + out <- expand(data, ...) names <- names(out) From 2952738e1792cb62f391a52e808b86b193bc32cc Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 18 Oct 2022 16:16:12 -0500 Subject: [PATCH 23/26] Minor polishing --- R/hoist.R | 5 +++-- R/unnest-helper.R | 2 +- R/unnest-longer.R | 4 ++-- tests/testthat/_snaps/unnest-longer.md | 4 ++-- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/R/hoist.R b/R/hoist.R index 6cd881885..a9307e6b1 100644 --- a/R/hoist.R +++ b/R/hoist.R @@ -89,10 +89,11 @@ hoist <- function(.data, x <- .data[[.col]] vec_check_list(x, arg = ".data[[.col]]") - # Check these here to generate error message with correct variable name + # These are also checked in df_simplify(), but we check here to generate + # errors with argument names check_list_of_ptypes(.ptype, names(x)) - check_list_of_functions(.transform, names(x)) check_list_of_bool(.simplify, names(x)) + check_list_of_functions(.transform, names(x)) # In R <4.1, `::` is quite slow and this is a tight loop, so eliminating # the lookup has a large performance impact: diff --git a/R/unnest-helper.R b/R/unnest-helper.R index 04d94a1d1..218f2856a 100644 --- a/R/unnest-helper.R +++ b/R/unnest-helper.R @@ -32,7 +32,7 @@ df_simplify <- function(x, x = col, ptype = col_ptype, transform = col_transform, - simplify = col_simplify %||% TRUE, + simplify = col_simplify, error_call = error_call ) } diff --git a/R/unnest-longer.R b/R/unnest-longer.R index 715faa8f8..8fa4b733d 100644 --- a/R/unnest-longer.R +++ b/R/unnest-longer.R @@ -91,7 +91,7 @@ unnest_longer <- function(data, } else { if (is_false(indices_include)) { cli::cli_abort( - "Can't set {.arg indices_include} to {.code FALSE} when {.arg indices_to} is supplied." + "Can't use {.code indices_include = FALSE} when {.arg indices_to} is supplied." ) } indices_include <- TRUE @@ -214,7 +214,7 @@ elt_to_long <- function(x, if (!vec_is(x)) { cli::cli_abort( - "Column {.var {name}} must contain a list of vectors.", + "List-column {.var {name}} must contain only vectors.", call = error_call ) } diff --git a/tests/testthat/_snaps/unnest-longer.md b/tests/testthat/_snaps/unnest-longer.md index 2cb4027c4..2d3dfe9be 100644 --- a/tests/testthat/_snaps/unnest-longer.md +++ b/tests/testthat/_snaps/unnest-longer.md @@ -5,7 +5,7 @@ Output Error in `unnest_longer()`: - ! Column `y` must contain a list of vectors. + ! List-column `y` must contain only vectors. # can't mix `indices_to` with `indices_include = FALSE` @@ -15,7 +15,7 @@ Output Error in `unnest_longer()`: - ! Can't set `indices_include` to `FALSE` when `indices_to` is supplied. + ! Can't use `indices_include = FALSE` when `indices_to` is supplied. # unnest_longer() validates its inputs From 680549bacdbc1b8631d2ae3b825e166a4339bacc Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 18 Oct 2022 16:17:11 -0500 Subject: [PATCH 24/26] Remove last use of stop() --- R/append.R | 2 +- tests/testthat/_snaps/append.md | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/R/append.R b/R/append.R index 9c60e9064..17134cb87 100644 --- a/R/append.R +++ b/R/append.R @@ -2,7 +2,7 @@ append_df <- function(x, y, after = length(x), remove = FALSE) { if (is.character(after)) { after <- match(after, dplyr::tbl_vars(x)) } else if (!is.integer(after)) { - stop("`after` must be character or integer", call. = FALSE) + cli::cli_abort("{.arg after} must be character or integer", .internal = TRUE) } # Replace duplicated variables diff --git a/tests/testthat/_snaps/append.md b/tests/testthat/_snaps/append.md index 4493731ae..22faf8ec0 100644 --- a/tests/testthat/_snaps/append.md +++ b/tests/testthat/_snaps/append.md @@ -3,5 +3,9 @@ Code (expect_error(append_df(df1, df2, after = 1))) Output - + + Error in `append_df()`: + ! `after` must be character or integer + i This is an internal error that was detected in the tidyr package. + Please report it at with a reprex () and the full backtrace. From 8a5524ef1a0d6a3b70d8086fb1a81c150dc94c29 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 18 Oct 2022 16:18:18 -0500 Subject: [PATCH 25/26] Add missing default --- R/unnest-helper.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/unnest-helper.R b/R/unnest-helper.R index 218f2856a..576261bb7 100644 --- a/R/unnest-helper.R +++ b/R/unnest-helper.R @@ -26,7 +26,7 @@ df_simplify <- function(x, col_ptype <- ptype[[col_name]] col_transform <- transform[[col_name]] - col_simplify <- simplify[[col_name]] + col_simplify <- simplify[[col_name]] %||% TRUE out[[i]] <- col_simplify( x = col, From 1e64914fb2788a6733aba428895fb1138b1c5e64 Mon Sep 17 00:00:00 2001 From: Hadley Wickham Date: Tue, 18 Oct 2022 16:46:48 -0500 Subject: [PATCH 26/26] Final replacements for abort() calls --- R/chop.R | 5 +- R/expand.R | 4 -- R/hoist.R | 2 +- R/nest-legacy.R | 10 +-- R/pivot-long.R | 27 ++++---- R/pivot-wide.R | 94 +++++++++++++-------------- R/replace_na.R | 5 +- R/spread.R | 7 +- R/unnest-wider.R | 5 +- R/utils.R | 18 +++-- tests/testthat/_snaps/nest-legacy.md | 3 +- tests/testthat/_snaps/pivot-long.md | 2 +- tests/testthat/_snaps/pivot-wide.md | 40 ++++++------ tests/testthat/_snaps/replace_na.md | 2 +- tests/testthat/_snaps/spread.md | 2 +- tests/testthat/_snaps/unnest-wider.md | 2 +- 16 files changed, 114 insertions(+), 114 deletions(-) diff --git a/R/chop.R b/R/chop.R index ce7d6c97a..e4dfc63e8 100644 --- a/R/chop.R +++ b/R/chop.R @@ -243,7 +243,10 @@ df_unchop <- function(x, ..., ptype = NULL, keep_empty = FALSE, error_call = cal # - `col` was an empty list(), or a list of all `NULL`s. # - No ptype was specified for `col`, either by the user or by a list-of. if (out_size != 0L) { - abort("`NULL` column generated, but output size is not `0`.", .internal = TRUE) + cli::cli_abort( + "`NULL` column generated, but output size is not `0`.", + .internal = TRUE + ) } col <- unspecified(0L) diff --git a/R/expand.R b/R/expand.R index d82ddb842..d2dd4c317 100644 --- a/R/expand.R +++ b/R/expand.R @@ -214,10 +214,6 @@ sorted_unique <- function(x) { # forcats::fct_unique fct_unique <- function(x) { - if (!is.factor(x)) { - abort("`x` must be a factor.") - } - levels <- levels(x) out <- levels diff --git a/R/hoist.R b/R/hoist.R index a9307e6b1..3f5140a71 100644 --- a/R/hoist.R +++ b/R/hoist.R @@ -157,7 +157,7 @@ check_pluckers <- function(..., .call = caller_env()) { strike <- function(x, indices) { if (!vec_is_list(indices)) { - abort("`indices` must be a list.", .internal = TRUE) + cli::cli_abort("{.arg indices} must be a list.", .internal = TRUE) } n_indices <- vec_size(indices) diff --git a/R/nest-legacy.R b/R/nest-legacy.R index 155566c8b..7fa5ccc21 100644 --- a/R/nest-legacy.R +++ b/R/nest-legacy.R @@ -147,16 +147,16 @@ unnest_legacy.data.frame <- function(data, ..., .drop = NA, .id = NULL, nested <- dplyr::transmute(dplyr::ungroup(data), !!!quos) n <- map(nested, function(x) unname(map_int(x, NROW))) if (length(unique(n)) != 1) { - abort("All nested columns must have the same number of elements.") + cli::cli_abort("All nested columns must have the same number of elements.") } types <- map_chr(nested, list_col_type) nest_types <- split.default(nested, types) if (length(nest_types$mixed) > 0) { - probs <- paste(names(nest_types$mixed), collapse = ",") - abort(glue( - "Each column must either be a list of vectors or a list of ", - "data frames [{probs}]" + probs <- names(nest_types$mixed) + cli::cli_abort(c( + "Each column must either be a list of vectors or a list of data frames.", + i = "Problems in: {.var probs}" )) } diff --git a/R/pivot-long.R b/R/pivot-long.R index be0a3b841..baf0a31e5 100644 --- a/R/pivot-long.R +++ b/R/pivot-long.R @@ -248,8 +248,7 @@ pivot_longer_spec <- function(data, cols_vary <- arg_match0( arg = cols_vary, - values = c("fastest", "slowest"), - arg_nm = "cols_vary" + values = c("fastest", "slowest") ) # Quick hack to ensure that split() preserves order @@ -296,7 +295,7 @@ pivot_longer_spec <- function(data, } else if (cols_vary == "fastest") { vals[[value]] <- vec_interleave(!!!val_cols, .ptype = val_type) } else { - abort("Unknown `cols_vary` value.", .internal = TRUE) + cli::cli_abort("Unknown {arg cols_vary} value.", .internal = TRUE) } } vals <- as_tibble(vals) @@ -314,7 +313,7 @@ pivot_longer_spec <- function(data, data_cols <- vec_rep_each(data_cols, times_data_cols) keys <- vec_rep(keys, times_keys) } else { - abort("Unknown `cols_vary` value.", .internal = TRUE) + cli::cli_abort("Unknown {arg cols_vary} value.", .internal = TRUE) } out <- wrap_error_names(vec_cbind( @@ -345,11 +344,16 @@ build_longer_spec <- function(data, names_pattern = NULL, names_ptypes = NULL, names_transform = NULL) { + + check_data_frame(data) + check_required(cols) + check_character(names_to, allow_null = TRUE) + cols <- tidyselect::eval_select(enquo(cols), data[unique(names(data))], allow_rename = FALSE) cols <- names(cols) if (length(cols) == 0) { - abort(glue::glue("`cols` must select at least one column.")) + cli::cli_abort("{.arg cols} must select at least one column.") } if (is.null(names_prefix)) { @@ -361,9 +365,6 @@ build_longer_spec <- function(data, if (is.null(names_to)) { names_to <- character(0L) } - if (!is.character(names_to)) { - abort("`names_to` must be a character vector or `NULL`.") - } n_names_to <- length(names_to) has_names_sep <- !is.null(names_sep) @@ -373,7 +374,7 @@ build_longer_spec <- function(data, names <- tibble::new_tibble(x = list(), nrow = length(names)) } else if (n_names_to == 1L) { if (has_names_sep) { - abort("`names_sep` can't be used with a length 1 `names_to`.") + cli::cli_abort("{.arg names_sep} can't be used with a length 1 {.arg names_to}.") } if (has_names_pattern) { names <- str_extract(names, names_to, regex = names_pattern)[[1]] @@ -382,9 +383,9 @@ build_longer_spec <- function(data, names <- tibble(!!names_to := names) } else { if (!xor(has_names_sep, has_names_pattern)) { - abort(glue::glue( - "If you supply multiple names in `names_to` you must also supply one", - " of `names_sep` or `names_pattern`." + cli::cli_abort(paste0( + "If you supply multiple names in {.arg names_to} you must also supply one", + " of {.arg names_sep} or {.arg names_pattern}." )) } @@ -432,7 +433,7 @@ drop_cols <- function(df, cols) { } else if (is.integer(cols)) { df[-cols] } else { - abort("Invalid input") + cli::cli_abort("Invalid input", .internal = TRUE) } } diff --git a/R/pivot-wide.R b/R/pivot-wide.R index ce0035637..9ad46680f 100644 --- a/R/pivot-wide.R +++ b/R/pivot-wide.R @@ -285,9 +285,9 @@ pivot_wider_spec <- function(data, values_fill = NULL, values_fn = NULL, unused_fn = NULL) { - input <- data spec <- check_pivot_spec(spec) + check_bool(id_expand) names_from_cols <- names(spec)[-(1:2)] values_from_cols <- vec_unique(spec$.value) @@ -307,19 +307,14 @@ pivot_wider_spec <- function(data, if (is.null(values_fill)) { values_fill <- list() - } - if (is_scalar(values_fill)) { + } else if (is_scalar(values_fill)) { values_fill <- rep_named(values_from_cols, list(values_fill)) - } - if (!vec_is_list(values_fill)) { - abort("`values_fill` must be NULL, a scalar, or a named list") + } else if (!vec_is_list(values_fill)) { + cli::cli_abort("{arg values_fill} must be NULL, a scalar, or a named list, not a {.obj_type_friendly {values_fill}") } values_fill <- values_fill[intersect(names(values_fill), values_from_cols)] - if (!is_bool(id_expand)) { - abort("`id_expand` must be a single `TRUE` or `FALSE`.") - } - + input <- data # Early conversion to tibble because data.table returns zero rows if # zero cols are selected. Also want to avoid the grouped-df behavior # of `complete()`. @@ -422,15 +417,15 @@ pivot_wider_spec <- function(data, group_cols <- backtick_if_not_syntactic(group_cols) group_cols <- glue::glue_collapse(group_cols, sep = ", ") - warn(glue::glue( - "Values from {duplicate_names} are not uniquely identified; output will contain list-cols.\n", - "* Use `values_fn = list` to suppress this warning.\n", - "* Use `values_fn = {{summary_fun}}` to summarise duplicates.\n", - "* Use the following dplyr code to identify duplicates.\n", - " {{data}} %>%\n", - " dplyr::group_by({group_cols}) %>%\n", - " dplyr::summarise(n = dplyr::n(), .groups = \"drop\") %>%\n", - " dplyr::filter(n > 1L)" + cli::cli_warn(c( + "Values from {duplicate_names} are not uniquely identified; output will contain list-cols.", + "*" = "Use `values_fn = list` to suppress this warning.", + "*" = "Use `values_fn = {{summary_fun}}` to summarise duplicates.", + "*" = "Use the following dplyr code to identify duplicates.", + " " = " {{data}} %>%", + " " = " dplyr::group_by({group_cols}) %>%", + " " = " dplyr::summarise(n = dplyr::n(), .groups = \"drop\") %>%", + " " = " dplyr::filter(n > 1L)" )) } @@ -463,21 +458,25 @@ build_wider_spec <- function(data, names_sort = FALSE, names_vary = "fastest", names_expand = FALSE) { - names_from <- tidyselect::eval_select(enquo(names_from), data, allow_rename = FALSE) - values_from <- tidyselect::eval_select(enquo(values_from), data, allow_rename = FALSE) - - if (is_empty(names_from)) { - abort("`names_from` must select at least one column.") - } - if (is_empty(values_from)) { - abort("`values_from` must select at least one column.") - } + names_from <- tidyselect::eval_select( + enquo(names_from), + data, + allow_rename = FALSE, + allow_empty = FALSE + ) + values_from <- tidyselect::eval_select( + enquo(values_from), + data, + allow_rename = FALSE, + allow_empty = FALSE + ) + check_string(names_prefix) + check_string(names_sep) + check_string(names_glue, allow_null = TRUE) + check_bool(names_sort) names_vary <- arg_match0(names_vary, c("fastest", "slowest"), arg_nm = "names_vary") - - if (!is_bool(names_expand)) { - abort("`names_expand` must be a single `TRUE` or `FALSE`.") - } + check_bool(names_expand) data <- as_tibble(data) data <- data[names_from] @@ -528,8 +527,6 @@ build_wider_id_cols_expr <- function(data, names_from = name, values_from = value, error_call = caller_env()) { - # TODO: Use `allow_rename = FALSE`. - # Requires https://github.com/r-lib/tidyselect/issues/225. names_from <- tidyselect::eval_select( enquo(names_from), data, @@ -588,9 +585,7 @@ select_wider_id_cols <- function(data, rethrow_id_cols_oob <- function(cnd, names_from_cols, values_from_cols, call) { i <- cnd[["i"]] - if (!is_string(i)) { - abort("`i` is expected to be a string.", .internal = TRUE) - } + check_string(i, .internal = TRUE) if (i %in% names_from_cols) { stop_id_cols_oob(i, "names_from", call = call) @@ -603,11 +598,14 @@ rethrow_id_cols_oob <- function(cnd, names_from_cols, values_from_cols, call) { } stop_id_cols_oob <- function(i, arg, call) { - message <- c( - glue("`id_cols` can't select a column already selected by `{arg}`."), - i = glue("Column `{i}` has already been selected.") + cli::cli_abort( + c( + "`id_cols` can't select a column already selected by `{arg}`.", + i = "Column `{i}` has already been selected." + ), + parent = NA, + call = call ) - abort(message, parent = NA, call = call) } # Helpers ----------------------------------------------------------------- @@ -628,15 +626,13 @@ value_summarize <- function(value, value_locs, value_name, fn, fn_name, error_ca if (any(invalid_sizes)) { size <- sizes[invalid_sizes][[1]] - header <- glue( - "Applying `{fn_name}` to `{value_name}` must result in ", - "a single summary value per key." - ) - bullet <- c( - x = glue("Applying `{fn_name}` resulted in a value with length {size}.") + cli::cli_abort( + c( + "Applying {.arg {fn_name}} to {.var {value_name}} must result in a single summary value per key.", + i = "Applying {.arg {fn_name}} resulted in a vector of length {size}." + ), + call = error_call ) - - abort(c(header, bullet), call = error_call) } value <- vec_c(!!!value) diff --git a/R/replace_na.R b/R/replace_na.R index c9076f9e1..119c9b01c 100644 --- a/R/replace_na.R +++ b/R/replace_na.R @@ -81,6 +81,9 @@ check_replacement <- function(x, var, call = caller_env()) { n <- vec_size(x) if (n != 1) { - abort(glue("Replacement for `{var}` is length {n}, not length 1."), call = call) + cli::cli_abort( + "Replacement for `{var}` must be length 1, not length {n}.", + call = call + ) } } diff --git a/R/spread.R b/R/spread.R index 2358e2dc1..1f7bf4d7d 100644 --- a/R/spread.R +++ b/R/spread.R @@ -95,11 +95,10 @@ spread.data.frame <- function(data, key, value, fill = NA, convert = FALSE, shared <- sum(map_int(groups, length)) str <- map_chr(groups, function(x) paste0(x, collapse = ", ")) - rows <- paste0(vec_paste0("* ", str, "\n"), collapse = "") - abort(glue( + cli::cli_abort(c( "Each row of output must be identified by a unique combination of keys.", - "\nKeys are shared for {shared} rows:", - "\n{rows}" + i = "Keys are shared for {shared} rows", + set_names(str, "*") )) } diff --git a/R/unnest-wider.R b/R/unnest-wider.R index 663fcd113..23117ed11 100644 --- a/R/unnest-wider.R +++ b/R/unnest-wider.R @@ -176,7 +176,10 @@ elt_to_wide <- function(x, name, strict, names_sep, error_call = caller_env()) { } if (!vec_is(x)) { - abort(glue("Column `{name}` must contain a list of vectors."), call = error_call) + cli::cli_abort( + "List-column {.var {name}} must contain only vectors.", + call = error_call + ) } if (is.data.frame(x)) { diff --git a/R/utils.R b/R/utils.R index 3e2d6705e..077b5fee7 100644 --- a/R/utils.R +++ b/R/utils.R @@ -78,13 +78,12 @@ tidyr_col_modify <- function(data, cols) { # data frame methods for `[<-` and `[[<-`. # Assume each element of `cols` has the correct size. - if (!is.data.frame(data)) { - abort("Internal error: `data` must be a data frame.") - } + check_data_frame(data, .internal = TRUE) if (!is_list(cols)) { - abort("Internal error: `cols` must be a list.") + cli::cli_abort("`cols` must be a list.", .internal = TRUE) } + size <- vec_size(data) data <- tidyr_new_list(data) @@ -104,7 +103,7 @@ tidyr_col_modify <- function(data, cols) { tidyr_new_list <- function(x) { if (!is_list(x)) { - abort("Internal error: `x` must be a VECSXP.") + cli::cli_abort("`x` must be a list.", .internal = TRUE) } names <- names(x) @@ -131,9 +130,8 @@ list_init_empty <- function(x, null = TRUE, typed = TRUE) { check_dots_empty() - - if (!vec_is_list(x)) { - abort("Internal error: `x` must be a list.") + if (!is_list(x)) { + cli::cli_abort("`x` must be a list.", .internal = TRUE) } sizes <- list_sizes(x) @@ -193,9 +191,9 @@ vec_paste0 <- function(...) { exec(paste0, !!!args) } -check_data_frame <- function(x, arg = caller_arg(x), call = caller_env()) { +check_data_frame <- function(x, ..., arg = caller_arg(x), call = caller_env()) { if (!is.data.frame(x)) { - cli::cli_abort("{.arg {arg}} must be a data frame, not {.obj_type_friendly {x}}.", call = call) + cli::cli_abort("{.arg {arg}} must be a data frame, not {.obj_type_friendly {x}}.", ..., call = call) } } diff --git a/tests/testthat/_snaps/nest-legacy.md b/tests/testthat/_snaps/nest-legacy.md index da12a349d..d4638c4f0 100644 --- a/tests/testthat/_snaps/nest-legacy.md +++ b/tests/testthat/_snaps/nest-legacy.md @@ -5,7 +5,8 @@ Output Error in `unnest_legacy()`: - ! Each column must either be a list of vectors or a list of data frames [x] + ! Each column must either be a list of vectors or a list of data frames. + i Problems in: `probs` # multiple columns must be same length diff --git a/tests/testthat/_snaps/pivot-long.md b/tests/testthat/_snaps/pivot-long.md index a3b2a7b59..2476cc6c5 100644 --- a/tests/testthat/_snaps/pivot-long.md +++ b/tests/testthat/_snaps/pivot-long.md @@ -88,7 +88,7 @@ Output Error in `build_longer_spec()`: - ! `names_to` must be a character vector or `NULL`. + ! `names_to` must be a character vector or `NULL`, not the number 1. Code (expect_error(build_longer_spec(df, x, names_to = c("x", "y")))) Output diff --git a/tests/testthat/_snaps/pivot-wide.md b/tests/testthat/_snaps/pivot-wide.md index b42f86859..a6feb9dd0 100644 --- a/tests/testthat/_snaps/pivot-wide.md +++ b/tests/testthat/_snaps/pivot-wide.md @@ -47,7 +47,7 @@ Output Error in `build_wider_spec()`: - ! `names_from` must select at least one column. + ! Must select at least one item. # `values_from` must identify at least 1 column (#1240) @@ -57,7 +57,7 @@ Output Error in `build_wider_spec()`: - ! `values_from` must select at least one column. + ! Must select at least one item. # `values_fn` emits an informative error when it doesn't result in unique values (#1238) @@ -67,7 +67,7 @@ Error in `pivot_wider_spec()`: ! Applying `values_fn` to `value` must result in a single summary value per key. - x Applying `values_fn` resulted in a value with length 2. + i Applying `values_fn` resulted in a vector of length 2. # `names_vary` is validated @@ -91,13 +91,13 @@ Output Error in `build_wider_spec()`: - ! `names_expand` must be a single `TRUE` or `FALSE`. + ! `names_expand` must be `TRUE` or `FALSE`, not the number 1. Code (expect_error(build_wider_spec(df, names_expand = "x"))) Output Error in `build_wider_spec()`: - ! `names_expand` must be a single `TRUE` or `FALSE`. + ! `names_expand` must be `TRUE` or `FALSE`, not the string "x". # `id_cols` can't select columns from `names_from` or `values_from` (#1318) @@ -143,13 +143,13 @@ Output Error in `pivot_wider_spec()`: - ! `id_expand` must be a single `TRUE` or `FALSE`. + ! `id_expand` must be `TRUE` or `FALSE`, not the number 1. Code (expect_error(pivot_wider(df, id_expand = "x"))) Output Error in `pivot_wider_spec()`: - ! `id_expand` must be a single `TRUE` or `FALSE`. + ! `id_expand` must be `TRUE` or `FALSE`, not the string "x". # duplicated keys produce list column with warning @@ -162,9 +162,9 @@ * Use `values_fn = {summary_fun}` to summarise duplicates. * Use the following dplyr code to identify duplicates. {data} %>% - dplyr::group_by(a, key) %>% - dplyr::summarise(n = dplyr::n(), .groups = "drop") %>% - dplyr::filter(n > 1L) + dplyr::group_by(a, key) %>% + dplyr::summarise(n = dplyr::n(), .groups = "drop") %>% + dplyr::filter(n > 1L) # duplicated key warning mentions every applicable column @@ -177,9 +177,9 @@ * Use `values_fn = {summary_fun}` to summarise duplicates. * Use the following dplyr code to identify duplicates. {data} %>% - dplyr::group_by(key) %>% - dplyr::summarise(n = dplyr::n(), .groups = "drop") %>% - dplyr::filter(n > 1L) + dplyr::group_by(key) %>% + dplyr::summarise(n = dplyr::n(), .groups = "drop") %>% + dplyr::filter(n > 1L) Output # A tibble: 1 x 3 a_x b_x c_x @@ -197,9 +197,9 @@ * Use `values_fn = {summary_fun}` to summarise duplicates. * Use the following dplyr code to identify duplicates. {data} %>% - dplyr::group_by(key) %>% - dplyr::summarise(n = dplyr::n(), .groups = "drop") %>% - dplyr::filter(n > 1L) + dplyr::group_by(key) %>% + dplyr::summarise(n = dplyr::n(), .groups = "drop") %>% + dplyr::filter(n > 1L) Output # A tibble: 1 x 3 a_x b_x c_x @@ -217,9 +217,9 @@ * Use `values_fn = {summary_fun}` to summarise duplicates. * Use the following dplyr code to identify duplicates. {data} %>% - dplyr::group_by(`a 1`, a2, `the-key`) %>% - dplyr::summarise(n = dplyr::n(), .groups = "drop") %>% - dplyr::filter(n > 1L) + dplyr::group_by(`a 1`, a2, `the-key`) %>% + dplyr::summarise(n = dplyr::n(), .groups = "drop") %>% + dplyr::filter(n > 1L) # values_fn is validated @@ -238,7 +238,7 @@ Error in `pivot_wider_spec()`: ! Applying `unused_fn` to `unused` must result in a single summary value per key. - x Applying `unused_fn` resulted in a value with length 2. + i Applying `unused_fn` resulted in a vector of length 2. # `unused_fn` is validated diff --git a/tests/testthat/_snaps/replace_na.md b/tests/testthat/_snaps/replace_na.md index 6d784fa3d..1614e0288 100644 --- a/tests/testthat/_snaps/replace_na.md +++ b/tests/testthat/_snaps/replace_na.md @@ -5,7 +5,7 @@ Output Error in `replace_na()`: - ! Replacement for `data` is length 10, not length 1. + ! Replacement for `data` must be length 1, not length 10. # replacement must be castable to `data` diff --git a/tests/testthat/_snaps/spread.md b/tests/testthat/_snaps/spread.md index 1d2ac15c1..593d5d7a0 100644 --- a/tests/testthat/_snaps/spread.md +++ b/tests/testthat/_snaps/spread.md @@ -6,6 +6,6 @@ Error in `spread()`: ! Each row of output must be identified by a unique combination of keys. - Keys are shared for 2 rows: + i Keys are shared for 2 rows * 2, 3 diff --git a/tests/testthat/_snaps/unnest-wider.md b/tests/testthat/_snaps/unnest-wider.md index 60b0217a8..407b1b83a 100644 --- a/tests/testthat/_snaps/unnest-wider.md +++ b/tests/testthat/_snaps/unnest-wider.md @@ -5,7 +5,7 @@ Output Error in `unnest_wider()`: - ! Column `y` must contain a list of vectors. + ! List-column `y` must contain only vectors. # can unnest a vector with a mix of named/unnamed elements (#1200 comment)