Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat: Prefer cli::cli_abort() over stop() or rlang::abort() #114

Merged
merged 25 commits into from
Mar 8, 2024
6 changes: 3 additions & 3 deletions R/as_duckplyr_df.R
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,15 @@ as_duckplyr_df <- function(.data) {
}

if (!identical(class(.data), "data.frame") && !identical(class(.data), c("tbl_df", "tbl", "data.frame"))) {
abort("Must pass a plain data frame or a tibble to `as_duckplyr_df()`.")
cli_abort("Must pass a plain data frame or a tibble to `as_duckplyr_df()`.")
}

if (is.character(.row_names_info(.data, 0L))) {
abort("Must pass data frame without row names to `as_duckplyr_df()`.")
cli_abort("Must pass data frame without row names to `as_duckplyr_df()`.")
}

if (anyNA(names(.data)) || any(names(.data) == "")) {
abort("Missing or empty names not allowed.")
cli_abort("Missing or empty names not allowed.")
}

class(.data) <- c("duckplyr_df", class(.data))
Expand Down
2 changes: 1 addition & 1 deletion R/count.R
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ count.duckplyr_df <- function(x, ..., wt = NULL, sort = FALSE, name = NULL, .dro
name <- check_n_name(name, by_chr)

if (name %in% by_chr) {
abort("Name clash in count()")
cli_abort("Name clash in `count()`")
}

n <- tally_n(x, {{ wt }})
Expand Down
13 changes: 6 additions & 7 deletions R/meta.R
Original file line number Diff line number Diff line change
Expand Up @@ -203,13 +203,12 @@ meta_rel_get <- function(rel) {
hash <- deparse(rel)

if (!rel_cache$has(hash)) {
abort(
c(
"duckplyr: internal: hash not found",
i = paste0("hash: ", hash),
i = paste0("relation: ", paste(utils::capture.output(print(rel), type = "message"), collapse = "\n"))
)
)
rel_out <- paste(utils::capture.output(print(rel), type = "message"), collapse = "\n")
cli_abort(c(
"duckplyr: internal: hash not found",
i = "hash: {hash}",
i = "relation: {rel_out}"
))
}

rel_cache$get(hash)
Expand Down
2 changes: 1 addition & 1 deletion R/oo.R
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ oo_prep <- function(
names <- rel_names(rel)

if (colname %in% names) {
abort("Must use column name not yet present in rel")
cli_abort("Can't use column {.var {colname}} already present in rel for order preservation")
}

proj_exprs <- imap(set_names(names), relexpr_reference, rel = NULL)
Expand Down
18 changes: 9 additions & 9 deletions R/relational-duckdb.R
Original file line number Diff line number Diff line change
Expand Up @@ -104,19 +104,19 @@ duckdb_rel_from_df <- function(df) {
# FIXME: This should be duckdb's responsibility
check_df_for_rel <- function(df) {
if (is.character(.row_names_info(df, 0L))) {
stop("Need data frame without row names to convert to relational.")
cli_abort("Need data frame without row names to convert to relational.")
}

for (i in seq_along(df)) {
col <- .subset2(df, i)
if (!is.null(names(col))) {
stop("Can't convert named vectors to relational. Affected column: `", names(df)[[i]], "`.")
cli_abort("Can't convert named vectors to relational. Affected column: {.var {names(df)[[i]]}}.")
}
if (!is.null(dim(col))) {
stop("Can't convert arrays or matrices to relational. Affected column: `", names(df)[[i]], "`.")
cli_abort("Can't convert arrays or matrices to relational. Affected column: {.var {names(df)[[i]]}}.")
}
if (isS4(col)) {
stop("Can't convert S4 columns to relational. Affected column: `", names(df)[[i]], "`.")
cli_abort("Can't convert S4 columns to relational. Affected column: {.var {names(df)[[i]]}}.")
}
# https://github.com/duckdb/duckdb/issues/8561
col_class <- class(col)
Expand All @@ -128,7 +128,7 @@ check_df_for_rel <- function(df) {
valid <- FALSE
}
if (!valid) {
stop("Can't convert columns of class ", paste0(col_class, collapse = "/"), " to relational. Affected column: `", names(df)[[i]], "`.")
cli_abort("Can't convert columns of class {.cls {paste0(col_class, collapse = '/')}} to relational. Affected column: {.var {names(df)[[i]]}}.")
}
}

Expand All @@ -145,7 +145,7 @@ check_df_for_rel <- function(df) {
rlang::with_options(duckdb.materialize_message = FALSE, {
for (i in seq_along(df)) {
if (!identical(df[[i]], roundtrip[[i]])) {
stop("Imperfect roundtrip. Affected column: `", names(df)[[i]], "`.")
cli_abort("Imperfect roundtrip. Affected column: {.var {names(df)[[i]]}}.")
}
}
})
Expand All @@ -154,7 +154,7 @@ check_df_for_rel <- function(df) {
df_attrib <- attributes(df[[i]])
roundtrip_attrib <- attributes(roundtrip[[i]])
if (!identical(df_attrib, roundtrip_attrib)) {
stop("Attributes are lost during conversion. Affected column: `", names(df)[[i]], "`.")
cli_abort("Attributes are lost during conversion. Affected column: {.var {names(df)[[i]]}}.")
}
}
}
Expand Down Expand Up @@ -406,7 +406,7 @@ to_duckdb_expr <- function(x) {
out
},
NULL = NULL,
stop("Unknown expr class: ", class(x)[[1]])
cli_abort("Unknown expr class: {.cls {paste(class(x), collapse = '/')}}")
)
}

Expand Down Expand Up @@ -482,6 +482,6 @@ to_duckdb_expr_meta <- function(x) {
out
},
NULL = expr(NULL),
stop("Unknown expr class: ", class(x)[[1]])
cli_abort("Unknown expr class: {.cls {paste(class(x), collapse = '/')}}")
)
}
31 changes: 14 additions & 17 deletions R/relational.R
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,11 @@ rel_try <- function(rel, ..., call = NULL) {

if (Sys.getenv("DUCKPLYR_TELEMETRY_TEST") == "TRUE") {
force(call)
abort(paste0(
call$name,
": ",
call_to_json(
error_cnd(message = paste0("Error in ", call$name)),
call
)
))
json <- call_to_json(
error_cnd(message = paste0("Error in ", call$name)),
call
)
cli_abort("{call$name}: {json}")
}

if (Sys.getenv("DUCKPLYR_FALLBACK_FORCE") == "TRUE") {
Expand All @@ -33,7 +30,7 @@ rel_try <- function(rel, ..., call = NULL) {
inform(message = c("Requested fallback for relational:", i = names(dots)[[i]]))
}
if (Sys.getenv("DUCKPLYR_FORCE") == "TRUE") {
abort("Fallback not available with DUCKPLYR_FORCE")
cli_abort("Fallback not available with {.envvar DUCKPLYR_FORCE}.")
}
}

Expand All @@ -58,7 +55,7 @@ rel_try <- function(rel, ..., call = NULL) {
}

# Never reached due to return() in code
stop("Must use a return() in rel_try().")
cli_abort("Must use a return() in rel_try().")
}

rel_translate_dots <- function(dots, data, forbid_new = FALSE) {
Expand Down Expand Up @@ -110,7 +107,7 @@ rel_translate <- function(
#
symbol = {
if (as.character(expr) %in% names_forbidden) {
abort(paste0("Can't reuse summary variable `", as.character(expr), "`."))
cli_abort("Can't reuse summary variable {.var {as.character(expr)}}.")
}
if (as.character(expr) %in% names_data) {
ref <- as.character(expr)
Expand Down Expand Up @@ -138,10 +135,10 @@ rel_translate <- function(
args <- as.list(call[-1])
bad <- !(names(args) %in% c("x"))
if (any(bad)) {
abort(paste0(name, "(", names(args)[which(bad)[[1]]], " = ) not supported"))
cli_abort("{name}({names(args)[which(bad)[[1]]]} = ) not supported")
}
if (!is.null(getOption("lubridate.week.start"))) {
abort('wday() with option("lubridate.week.start") not supported')
cli_abort('{.code wday()} with {.code option("lubridate.week.start")} not supported')
}
},
"strftime" = {
Expand All @@ -150,7 +147,7 @@ rel_translate <- function(
args <- as.list(call[-1])
bad <- !(names(args) %in% c("x", "format"))
if (any(bad)) {
abort(paste0(name, "(", names(args)[which(bad)[[1]]], " = ) not supported"))
cli_abort("{name}({names(args)[which(bad)[[1]]]} = ) not supported")
}
},
"%in%" = {
Expand All @@ -176,7 +173,7 @@ rel_translate <- function(
if (exists(var_name, envir = env)) {
return(do_translate(get(var_name, env), in_window = in_window))
} else {
abort(paste0("object `", var_name, "` not found"))
cli_abort("object {.var {var_name}} not found")
}
}
}
Expand Down Expand Up @@ -223,7 +220,7 @@ rel_translate <- function(
known <- c(names(duckplyr_macros), names(aliases), known_window, known_ops, known_funs)

if (!(name %in% known)) {
abort(paste0("Unknown function: ", name))
cli_abort("Unknown function: {.code {name}()}")
}

if (name %in% names(aliases)) {
Expand Down Expand Up @@ -269,7 +266,7 @@ rel_translate <- function(
fun
},
#
abort(paste0("Internal: Unknown type ", typeof(expr)))
cli_abort("Internal: Unknown type {.val {typeof(expr)}}")
)
}

Expand Down
2 changes: 1 addition & 1 deletion tests/testthat/_snaps/relational-duckdb.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
data.frame(a = vctrs::new_vctr(1:3)) %>% duckdb_rel_from_df()
Condition
Error in `check_df_for_rel()`:
! Can't convert columns of class vctrs_vctr to relational. Affected column: `a`.
! Can't convert columns of class <vctrs_vctr> to relational. Affected column: `a`.

# rel_aggregate()

Expand Down
Loading