From 4d964c41ffeb7d8b82d695bb0a29444de1093ea3 Mon Sep 17 00:00:00 2001 From: hrryt Date: Sat, 24 May 2025 01:28:11 +0100 Subject: [PATCH 1/5] Add helpful message when names_from/values_from default throws error --- R/pivot-wide.R | 83 +++++++++++++++++++++-------- tests/testthat/_snaps/pivot-wide.md | 4 ++ 2 files changed, 64 insertions(+), 23 deletions(-) diff --git a/R/pivot-wide.R b/R/pivot-wide.R index 511f2935..5245e409 100644 --- a/R/pivot-wide.R +++ b/R/pivot-wide.R @@ -196,20 +196,28 @@ pivot_wider.data.frame <- function( values_fn = NULL, unused_fn = NULL ) { + missing_name <- missing(names_from) + missing_value <- missing(values_from) + names_from <- enquo(names_from) values_from <- enquo(values_from) - spec <- build_wider_spec( - data = data, - names_from = !!names_from, - values_from = !!values_from, - names_prefix = names_prefix, - names_sep = names_sep, - names_glue = names_glue, - names_sort = names_sort, - names_vary = names_vary, - names_expand = names_expand, - error_call = current_env() + spec <- try_fetch( + build_wider_spec( + data = data, + names_from = !!names_from, + values_from = !!values_from, + names_prefix = names_prefix, + names_sep = names_sep, + names_glue = names_glue, + names_sort = names_sort, + names_vary = names_vary, + names_expand = names_expand, + error_call = current_env() + ), + vctrs_error_subscript_oob = function(cnd) { + rethrow_missing_cols_oob(cnd, missing_name, missing_value, current_env()) + } ) id_cols <- compat_id_cols( @@ -514,19 +522,31 @@ build_wider_spec <- function( ) { check_dots_empty0(...) - names_from <- tidyselect::eval_select( - enquo(names_from), - data, - allow_rename = FALSE, - allow_empty = FALSE, - error_call = error_call + missing_name <- missing(names_from) + missing_value <- missing(values_from) + vctrs_error_subscript_oob <- function(cnd) { + rethrow_missing_cols_oob(cnd, missing_name, missing_value, error_call) + } + + names_from <- try_fetch( + tidyselect::eval_select( + enquo(names_from), + data, + allow_rename = FALSE, + allow_empty = FALSE, + error_call = error_call + ), + vctrs_error_subscript_oob = vctrs_error_subscript_oob ) - values_from <- tidyselect::eval_select( - enquo(values_from), - data, - allow_rename = FALSE, - allow_empty = FALSE, - error_call = error_call + values_from <- try_fetch( + tidyselect::eval_select( + enquo(values_from), + data, + allow_rename = FALSE, + allow_empty = FALSE, + error_call = error_call + ), + vctrs_error_subscript_oob = vctrs_error_subscript_oob ) check_string(names_prefix, call = error_call) @@ -650,6 +670,23 @@ select_wider_id_cols <- function( names(id_cols) } +rethrow_missing_cols_oob <- function(cnd, missing_name, missing_value, call) { + i <- cnd[["i"]] + + check_string(i, .internal = TRUE) + + if (missing_name && i == "name") { + msg <- c(`!` = "Have you supplied {.var names_from}?") + cli::cli_abort(msg, parent = cnd, call = call) + } else if (missing_value && i == "value") { + msg <- c(`!` = "Have you supplied {.var values_from}?") + cli::cli_abort(msg, parent = cnd, call = call) + } else { + # Zap this special handler, throw the normal condition + zap() + } +} + rethrow_id_cols_oob <- function(cnd, names_from_cols, values_from_cols, call) { i <- cnd[["i"]] diff --git a/tests/testthat/_snaps/pivot-wide.md b/tests/testthat/_snaps/pivot-wide.md index 2658004c..62a3786e 100644 --- a/tests/testthat/_snaps/pivot-wide.md +++ b/tests/testthat/_snaps/pivot-wide.md @@ -32,6 +32,8 @@ pivot_wider(df, values_from = val) Condition Error in `pivot_wider()`: + ! Have you supplied `names_from`? + Caused by error in `pivot_wider()`: ! Can't select columns that don't exist. x Column `name` doesn't exist. @@ -41,6 +43,8 @@ pivot_wider(df, names_from = key) Condition Error in `pivot_wider()`: + ! Have you supplied `values_from`? + Caused by error in `pivot_wider()`: ! Can't select columns that don't exist. x Column `value` doesn't exist. From 2db99b8c0de1b42138e52932859c571b57a81385 Mon Sep 17 00:00:00 2001 From: hrryt Date: Sat, 24 May 2025 01:52:59 +0100 Subject: [PATCH 2/5] Add inform_missing() --- R/pivot-wide.R | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/R/pivot-wide.R b/R/pivot-wide.R index 5245e409..ea0b5738 100644 --- a/R/pivot-wide.R +++ b/R/pivot-wide.R @@ -676,17 +676,20 @@ rethrow_missing_cols_oob <- function(cnd, missing_name, missing_value, call) { check_string(i, .internal = TRUE) if (missing_name && i == "name") { - msg <- c(`!` = "Have you supplied {.var names_from}?") - cli::cli_abort(msg, parent = cnd, call = call) + inform_missing("names_from", parent = cnd, call = call) } else if (missing_value && i == "value") { - msg <- c(`!` = "Have you supplied {.var values_from}?") - cli::cli_abort(msg, parent = cnd, call = call) + inform_missing("values_from", parent = cnd, call = call) } else { # Zap this special handler, throw the normal condition zap() } } +inform_missing <- function(arg, parent, call) { + msg <- c(`!` = "Have you supplied {.var {arg}}?") + cli::cli_abort(msg, parent = parent, call = call) +} + rethrow_id_cols_oob <- function(cnd, names_from_cols, values_from_cols, call) { i <- cnd[["i"]] From 7caaee93c3e32c80f1c2a928bfa020d3e3042767 Mon Sep 17 00:00:00 2001 From: hrryt Date: Sat, 24 May 2025 02:24:41 +0100 Subject: [PATCH 3/5] Reword message --- R/pivot-wide.R | 8 ++++---- tests/testthat/_snaps/pivot-wide.md | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/R/pivot-wide.R b/R/pivot-wide.R index ea0b5738..879f30a7 100644 --- a/R/pivot-wide.R +++ b/R/pivot-wide.R @@ -676,17 +676,17 @@ rethrow_missing_cols_oob <- function(cnd, missing_name, missing_value, call) { check_string(i, .internal = TRUE) if (missing_name && i == "name") { - inform_missing("names_from", parent = cnd, call = call) + stop_inform_missing(i, "names_from", parent = cnd, call = call) } else if (missing_value && i == "value") { - inform_missing("values_from", parent = cnd, call = call) + stop_inform_missing(i, "values_from", parent = cnd, call = call) } else { # Zap this special handler, throw the normal condition zap() } } -inform_missing <- function(arg, parent, call) { - msg <- c(`!` = "Have you supplied {.var {arg}}?") +stop_inform_missing <- function(i, arg, parent, call) { + msg <- c(`i` = "Column {.var {i}} is the default for {.var {arg}}.") cli::cli_abort(msg, parent = parent, call = call) } diff --git a/tests/testthat/_snaps/pivot-wide.md b/tests/testthat/_snaps/pivot-wide.md index 62a3786e..349c993a 100644 --- a/tests/testthat/_snaps/pivot-wide.md +++ b/tests/testthat/_snaps/pivot-wide.md @@ -32,7 +32,7 @@ pivot_wider(df, values_from = val) Condition Error in `pivot_wider()`: - ! Have you supplied `names_from`? + i Column `name` is the default for `names_from`. Caused by error in `pivot_wider()`: ! Can't select columns that don't exist. x Column `name` doesn't exist. @@ -43,7 +43,7 @@ pivot_wider(df, names_from = key) Condition Error in `pivot_wider()`: - ! Have you supplied `values_from`? + i Column `value` is the default for `values_from`. Caused by error in `pivot_wider()`: ! Can't select columns that don't exist. x Column `value` doesn't exist. From 3d919f0ef25036d01873439d086026f9b08d29a8 Mon Sep 17 00:00:00 2001 From: hrryt Date: Sat, 24 May 2025 02:42:34 +0100 Subject: [PATCH 4/5] Clarify error message --- R/pivot-wide.R | 6 ++++-- tests/testthat/_snaps/pivot-wide.md | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/R/pivot-wide.R b/R/pivot-wide.R index 879f30a7..96a725f9 100644 --- a/R/pivot-wide.R +++ b/R/pivot-wide.R @@ -686,8 +686,10 @@ rethrow_missing_cols_oob <- function(cnd, missing_name, missing_value, call) { } stop_inform_missing <- function(i, arg, parent, call) { - msg <- c(`i` = "Column {.var {i}} is the default for {.var {arg}}.") - cli::cli_abort(msg, parent = parent, call = call) + cli::cli_abort( + c(`i` = "Column {.var {i}} is the default for the {.var {arg}} argument."), + parent = parent, call = call + ) } rethrow_id_cols_oob <- function(cnd, names_from_cols, values_from_cols, call) { diff --git a/tests/testthat/_snaps/pivot-wide.md b/tests/testthat/_snaps/pivot-wide.md index 349c993a..2087eb89 100644 --- a/tests/testthat/_snaps/pivot-wide.md +++ b/tests/testthat/_snaps/pivot-wide.md @@ -32,7 +32,7 @@ pivot_wider(df, values_from = val) Condition Error in `pivot_wider()`: - i Column `name` is the default for `names_from`. + i Column `name` is the default for the `names_from` argument. Caused by error in `pivot_wider()`: ! Can't select columns that don't exist. x Column `name` doesn't exist. @@ -43,7 +43,7 @@ pivot_wider(df, names_from = key) Condition Error in `pivot_wider()`: - i Column `value` is the default for `values_from`. + i Column `value` is the default for the `values_from` argument. Caused by error in `pivot_wider()`: ! Can't select columns that don't exist. x Column `value` doesn't exist. From 16e0ba861cdba63a93d9b0b282957235aec7e99a Mon Sep 17 00:00:00 2001 From: hrryt Date: Sat, 24 May 2025 10:31:17 +0100 Subject: [PATCH 5/5] Use .arg with cli --- R/pivot-wide.R | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/R/pivot-wide.R b/R/pivot-wide.R index 96a725f9..729a586e 100644 --- a/R/pivot-wide.R +++ b/R/pivot-wide.R @@ -687,7 +687,7 @@ rethrow_missing_cols_oob <- function(cnd, missing_name, missing_value, call) { stop_inform_missing <- function(i, arg, parent, call) { cli::cli_abort( - c(`i` = "Column {.var {i}} is the default for the {.var {arg}} argument."), + c(`i` = "Column {.var {i}} is the default for the {.arg {arg}} argument."), parent = parent, call = call ) }