diff --git a/ci/conda_env_r.txt b/ci/conda_env_r.txt index 03d5f3b625c8..0ac6b1879db3 100644 --- a/ci/conda_env_r.txt +++ b/ci/conda_env_r.txt @@ -15,7 +15,6 @@ # specific language governing permissions and limitations # under the License. -r-assertthat r-base r-bit64 r-dplyr diff --git a/r/DESCRIPTION b/r/DESCRIPTION index f682de189e72..54a1d00f894a 100644 --- a/r/DESCRIPTION +++ b/r/DESCRIPTION @@ -33,13 +33,12 @@ SystemRequirements: C++20; for AWS S3 support on Linux, libcurl and openssl, and cmake >= 3.26 (build-time only, and only for full source build) Biarch: true Imports: - assertthat, bit64 (>= 0.9-7), glue, methods, purrr, R6, - rlang (>= 1.0.0), + rlang (>= 1.2.0), stats, tidyselect (>= 1.0.0), utils, diff --git a/r/NAMESPACE b/r/NAMESPACE index 46c29b3e9370..b1648ba7f50b 100644 --- a/r/NAMESPACE +++ b/r/NAMESPACE @@ -428,8 +428,6 @@ export(write_parquet) export(write_to_raw) export(write_tsv_dataset) importFrom(R6,R6Class) -importFrom(assertthat,assert_that) -importFrom(assertthat,is.string) importFrom(bit64,integer64) importFrom(glue,glue) importFrom(methods,as) @@ -460,9 +458,14 @@ importFrom(rlang,as_quosure) importFrom(rlang,call2) importFrom(rlang,call_args) importFrom(rlang,call_name) +importFrom(rlang,caller_arg) importFrom(rlang,caller_env) +importFrom(rlang,check_bool) importFrom(rlang,check_dots_empty) importFrom(rlang,check_dots_empty0) +importFrom(rlang,check_number_decimal) +importFrom(rlang,check_number_whole) +importFrom(rlang,check_string) importFrom(rlang,dots_list) importFrom(rlang,dots_n) importFrom(rlang,enexpr) @@ -482,15 +485,22 @@ importFrom(rlang,is_bare_character) importFrom(rlang,is_bare_list) importFrom(rlang,is_call) importFrom(rlang,is_character) +importFrom(rlang,is_closure) importFrom(rlang,is_empty) +importFrom(rlang,is_environment) importFrom(rlang,is_false) importFrom(rlang,is_formula) importFrom(rlang,is_integerish) importFrom(rlang,is_interactive) importFrom(rlang,is_list) +importFrom(rlang,is_logical) +importFrom(rlang,is_missing) +importFrom(rlang,is_na) +importFrom(rlang,is_null) importFrom(rlang,is_quosure) importFrom(rlang,is_string) importFrom(rlang,is_symbol) +importFrom(rlang,is_vector) importFrom(rlang,list2) importFrom(rlang,new_data_mask) importFrom(rlang,new_environment) @@ -508,6 +518,7 @@ importFrom(rlang,quo_set_expr) importFrom(rlang,quos) importFrom(rlang,seq2) importFrom(rlang,set_names) +importFrom(rlang,stop_input_type) importFrom(rlang,sym) importFrom(rlang,syms) importFrom(rlang,trace_back) diff --git a/r/NEWS.md b/r/NEWS.md index c8588cb9b4ae..10078dd8e417 100644 --- a/r/NEWS.md +++ b/r/NEWS.md @@ -19,6 +19,9 @@ # arrow 25.0.0.9000 +## Minor improvements and fixes +- arrow no longer depends on assertthat and has impr (@olivroy, #50525). + # arrow 25.0.0 ## Breaking changes diff --git a/r/R/array.R b/r/R/array.R index f2b34fc03f8b..d9a4c9d93a9f 100644 --- a/r/R/array.R +++ b/r/R/array.R @@ -417,7 +417,9 @@ DictionaryArray$create <- function(x, dict = NULL) { return(Array$create(x)) } - assert_that(!is.null(dict)) + if (is.null(dict)) { + stop("dict cannot be `NULL`.", call. = FALSE) + } if (!is.Array(x)) { x <- Array$create(x) } @@ -450,18 +452,17 @@ StructArray$create <- function(...) { #' @export `[[.StructArray` <- function(x, i, ...) { + assert_is(i, c("character", "numeric", "integer")) if (is.character(i)) { x$GetFieldByName(i) } else if (is.numeric(i)) { x$field(i - 1) - } else { - stop("'i' must be character or numeric, not ", class(i), call. = FALSE) } } #' @export `$.StructArray` <- function(x, name, ...) { - assert_that(is.string(name)) + check_string(name) if (name %in% ls(x)) { get(name, x) } else { diff --git a/r/R/arrow-datum.R b/r/R/arrow-datum.R index 5ac72bf36856..fa6918b7ed83 100644 --- a/r/R/arrow-datum.R +++ b/r/R/arrow-datum.R @@ -29,9 +29,7 @@ ArrowDatum <- R6Class( call_function("cast", self, options = opts) }, SortIndices = function(descending = FALSE) { - assert_that(is.logical(descending)) - assert_that(length(descending) == 1L) - assert_that(!is.na(descending)) + check_bool(descending, allow_na = FALSE) call_function( "sort_indices", self, @@ -285,8 +283,7 @@ filter_rows <- function(x, i, keep_na = TRUE, ...) { #' @importFrom utils head #' @export head.ArrowDatum <- function(x, n = 6L, ...) { - assert_is(n, c("numeric", "integer")) - assert_that(length(n) == 1) + check_number_decimal(n) len <- NROW(x) if (n < 0) { # head(x, negative) means all but the last n rows @@ -306,8 +303,7 @@ head.ArrowDatum <- function(x, n = 6L, ...) { #' @importFrom utils tail #' @export tail.ArrowDatum <- function(x, n = 6L, ...) { - assert_is(n, c("numeric", "integer")) - assert_that(length(n) == 1) + check_number_decimal(n) if (!is.integer(n)) { n <- floor(n) } diff --git a/r/R/arrow-package.R b/r/R/arrow-package.R index 2706faee5cb1..3ca90bde0e0b 100644 --- a/r/R/arrow-package.R +++ b/r/R/arrow-package.R @@ -19,7 +19,6 @@ #' @importFrom R6 R6Class #' @importFrom purrr as_mapper map map2 map_chr map2_chr map_dbl map_dfr map_int map_lgl keep imap imap_chr #' @importFrom purrr compact flatten reduce walk -#' @importFrom assertthat assert_that is.string #' @importFrom rlang list2 %||% is_false abort dots_n warn enquo quo_is_null enquos is_integerish quos quo #' @importFrom rlang eval_tidy new_data_mask syms env new_environment env_bind set_names exec #' @importFrom rlang is_bare_character quo_get_expr quo_get_env quo_set_expr .data seq2 is_interactive @@ -27,7 +26,9 @@ #' @importFrom rlang is_list call2 is_empty as_function as_label arg_match is_symbol is_call call_args #' @importFrom rlang quo_set_env quo_get_env is_formula quo_is_call f_rhs parse_expr f_env new_quosure #' @importFrom rlang new_quosures expr_text caller_env check_dots_empty check_dots_empty0 dots_list is_string inform -#' @importFrom rlang is_bare_list call_name +#' @importFrom rlang is_bare_list call_name check_string check_number_whole check_number_decimal check_bool +#' @importFrom rlang is_logical caller_env caller_arg stop_input_type is_null is_closure +#' @importFrom rlang is_environment is_na is_vector is_missing #' @importFrom tidyselect vars_pull eval_select eval_rename #' @importFrom glue glue #' @importFrom bit64 integer64 diff --git a/r/R/arrow-tabular.R b/r/R/arrow-tabular.R index a83226a728c3..653da6f8ac54 100644 --- a/r/R/arrow-tabular.R +++ b/r/R/arrow-tabular.R @@ -36,26 +36,34 @@ ArrowTabular <- R6Class( if (is.integer(i)) { i <- Array$create(i) } - assert_that(is.Array(i)) + if (!is.Array(i)) { + stop("`i` must be an Array.") + } call_function("take", self, i) }, Filter = function(i, keep_na = TRUE) { if (is.logical(i)) { i <- Array$create(i) } - assert_that(is.Array(i, "bool")) + if (!is.Array(i, "bool")) { + stop("i must be an Array of bool", call. = FALSE) + } call_function("filter", self, i, options = list(keep_na = keep_na)) }, SortIndices = function(names, descending = FALSE) { - assert_that(is.character(names)) - assert_that(length(names) > 0) - assert_that(!anyNA(names)) + # if rlang adds allow_empty argument https://github.com/r-lib/rlang/pull/1745 + # use allow_empty = FALSE instead + check_character(names, allow_na = FALSE) + if (length(names) == 0) { + stop("names can't be empty.", call. = FALSE) + } if (length(descending) == 1L) { descending <- rep_len(descending, length(names)) } - assert_that(is.logical(descending)) - assert_that(identical(length(names), length(descending))) - assert_that(!anyNA(descending)) + check_logical(descending, allow_na = FALSE) + if (length(names) != length(descending)) { + stop("names and descending must have a common length", call. = FALSE) + } call_function( "sort_indices", self, @@ -142,18 +150,17 @@ as.data.frame.ArrowTabular <- function(x, row.names = NULL, optional = FALSE, .. #' @export `[[.ArrowTabular` <- function(x, i, ...) { + assert_is(i, c("character", "numeric")) if (is.character(i)) { x$GetColumnByName(i) } else if (is.numeric(i)) { x$column(i - 1) - } else { - stop("'i' must be character or numeric, not ", class(i), call. = FALSE) } } #' @export `$.ArrowTabular` <- function(x, name, ...) { - assert_that(is.string(name)) + check_string(name) if (name %in% ls(x)) { get(name, x) } else { @@ -163,10 +170,10 @@ as.data.frame.ArrowTabular <- function(x, row.names = NULL, optional = FALSE, .. #' @export `[[<-.ArrowTabular` <- function(x, i, value) { - if (!is.character(i) && !is.numeric(i)) { - stop("'i' must be character or numeric, not ", class(i), call. = FALSE) + assert_is(i, c("character", "numeric")) + if (length(i) != 1 || is.na(i)) { + stop("`i` must be length 1 and not NA", call. = FALSE) } - assert_that(length(i) == 1, !is.na(i)) if (is.null(value)) { if (is.character(i)) { @@ -209,7 +216,7 @@ as.data.frame.ArrowTabular <- function(x, row.names = NULL, optional = FALSE, .. #' @export `$<-.ArrowTabular` <- function(x, i, value) { - assert_that(is.string(i)) + check_string(i) # We need to check if `i` is in names in case it is an active binding (e.g. # `metadata`, in which case we use assign to change the active binding instead # of the column in the table) diff --git a/r/R/compression.R b/r/R/compression.R index 4cd50d47a06f..3477f9cbd849 100644 --- a/r/R/compression.R +++ b/r/R/compression.R @@ -47,7 +47,7 @@ Codec <- R6Class( ) ) Codec$create <- function(type = "gzip", compression_level = NA) { - if (is.string(type)) { + if (is_string(type)) { type <- util___Codec__Create( compression_from_name(type), compression_level @@ -104,7 +104,7 @@ compression_from_name <- function(name) { CompressedOutputStream <- R6Class("CompressedOutputStream", inherit = OutputStream) CompressedOutputStream$create <- function(stream, codec = "gzip", compression_level = NA) { codec <- Codec$create(codec, compression_level = compression_level) - if (is.string(stream)) { + if (is_string(stream)) { stream <- FileOutputStream$create(stream) } assert_is(stream, "OutputStream") @@ -118,7 +118,7 @@ CompressedOutputStream$create <- function(stream, codec = "gzip", compression_le CompressedInputStream <- R6Class("CompressedInputStream", inherit = InputStream) CompressedInputStream$create <- function(stream, codec = "gzip", compression_level = NA) { codec <- Codec$create(codec, compression_level = compression_level) - if (is.string(stream)) { + if (is_string(stream)) { stream <- ReadableFile$create(stream) } assert_is(stream, "InputStream") diff --git a/r/R/compute.R b/r/R/compute.R index d5da9024bf46..b21d820127ac 100644 --- a/r/R/compute.R +++ b/r/R/compute.R @@ -44,8 +44,10 @@ #' @include chunked-array.R #' @include scalar.R call_function <- function(function_name, ..., args = list(...), options = empty_named_list()) { - assert_that(is.string(function_name)) - assert_that(is.list(options), !is.null(names(options))) + check_string(function_name) + if (!is.list(options) || is.null(names(options))) { + stop("options must be a named list.", call. = FALSE) + } datum_classes <- c("Array", "ChunkedArray", "RecordBatch", "Table", "Scalar") valid_args <- map_lgl(args, ~ inherits(., datum_classes)) @@ -152,7 +154,9 @@ collect_arrays_from_dots <- function(dots) { return(dots[[1]]) } - assert_that(all(map_lgl(dots, is.Array))) + if (!all(map_lgl(dots, is.Array))) { + stop("dots must all be Arrays.", call. = FALSE) + } arrays <- unlist(lapply(dots, function(x) { if (inherits(x, "ChunkedArray")) { x$chunks @@ -176,8 +180,10 @@ quantile.ArrowDatum <- function( x <- Array$create(x) } assert_is(probs, c("numeric", "integer")) - assert_that(length(probs) > 0) - assert_that(all(probs >= 0 & probs <= 1)) + # if check_numbers_decimal() gets added in rlang, https://github.com/r-lib/rlang/issues/1714 + if (length(probs) == 0 || !all(probs >= 0 & probs <= 1)) { + stop("probs must be a probability vector between 0 and 1.") + } if (!na.rm && x$null_count > 0) { stop("Missing values not allowed if 'na.rm' is FALSE", call. = FALSE) } diff --git a/r/R/csv.R b/r/R/csv.R index a0ecd48677a8..29362219797d 100644 --- a/r/R/csv.R +++ b/r/R/csv.R @@ -440,7 +440,7 @@ csv_read_options <- function( encoding = "UTF-8", skip_rows_after_names = 0L ) { - assert_that(is.string(encoding)) + check_string(encoding) options <- csv___ReadOptions__initialize( list( @@ -646,14 +646,14 @@ csv_write_options <- function( quoting_style_opts <- c("Needed", "AllValid", "None") quoting_style <- match(quoting_style, quoting_style_opts) - 1L - assert_that(is.logical(include_header)) - assert_that(is_integerish(batch_size, n = 1, finite = TRUE), batch_size > 0) - assert_that(is.character(delimiter)) - assert_that(is.character(null_string)) - assert_that(!is.na(null_string)) - assert_that(length(null_string) == 1) - assert_that(!grepl('"', null_string), msg = "na argument must not contain quote characters.") - assert_that(is.character(eol)) + check_bool(include_header) + check_number_whole(batch_size, min = 1, allow_infinite = FALSE) + check_string(delimiter) + check_string(null_string, allow_na = FALSE) + if (grepl('"', null_string)) { + stop("na argument must not contain quote characters.", call. = FALSE) + } + check_string(eol) csv___WriteOptions__initialize( list( @@ -930,17 +930,17 @@ write_csv_arrow <- function( write_options = NULL, ... ) { - unsupported_passed_args <- names(list(...)) - - if (length(unsupported_passed_args)) { - stop( - "The following ", - ngettext(length(unsupported_passed_args), "argument is ", "arguments are "), - "not yet supported in Arrow: ", - oxford_paste(unsupported_passed_args), - call. = FALSE + # every other argument is not supported in arrow and inform those arguments + # are not yet supported in arrow. + check_dots_empty(error = function(cnd) { + rlang::abort( + c( + "Arguments not yet supported in Arrow", + conditionMessage(cnd) + ), + call = call("write_csv_arrow") ) - } + }) if (!missing(file) && !missing(sink)) { stop( diff --git a/r/R/dataset-factory.R b/r/R/dataset-factory.R index 02b2b8553c19..9e54c17ed94a 100644 --- a/r/R/dataset-factory.R +++ b/r/R/dataset-factory.R @@ -233,18 +233,14 @@ FileSystemDatasetFactory$create <- function( factory_options = list() ) { assert_is(filesystem, "FileSystem") - is.null(selector) || assert_is(selector, "FileSelector") - is.null(paths) || assert_is(paths, "character") - assert_that( - xor(is.null(selector), is.null(paths)), - msg = "Either selector or paths must be specified" - ) + assert_is(selector, "FileSelector", allow_null = TRUE) + assert_is(paths, "character", allow_null = TRUE) + if (!xor(is.null(selector), is.null(paths))) { + stop("Either selector or paths must be specified") + } assert_is(format, "FileFormat") if (!is.null(paths)) { - assert_that( - is.null(partitioning), - msg = "Partitioning not supported with paths" - ) + check_null(partitioning, msg = "Partitioning not supported with paths") # Validate that exclude_invalid_files is only option provided # All other options are only relevant for the FileSelector method invalid_opts <- setdiff(names(factory_options), "exclude_invalid_files") @@ -285,15 +281,14 @@ fsf_options <- function(factory_options, partitioning) { if (!is.null(factory_options$partition_base_dir)) { if ( inherits(partitioning, "HivePartitioning") || - (inherits(partitioning, "PartitioningFactory") && - identical(partitioning$type_name, "hive")) + (inherits(partitioning, "PartitioningFactory") && identical(partitioning$type_name, "hive")) ) { warning( "factory_options$partition_base_dir is not meaningful for Hive partitioning", call. = FALSE ) } else { - assert_that(is.string(factory_options$partition_base_dir)) + check_string(factory_options$partition_base_dir) } } diff --git a/r/R/dataset-scan.R b/r/R/dataset-scan.R index c4b651d419dd..d56aa5c64e6f 100644 --- a/r/R/dataset-scan.R +++ b/r/R/dataset-scan.R @@ -160,10 +160,8 @@ names.Scanner <- function(x) names(x$schema) #' @export head.Scanner <- function(x, n = 6L, ...) { - assert_is(n, c("numeric", "integer")) - assert_that(length(n) == 1) # Negative n requires knowing nrow(x), which requires a scan itself - assert_that(n >= 0) + check_number_decimal(n, min = 0) if (!is.integer(n)) { n <- floor(n) } @@ -176,10 +174,8 @@ tail.Scanner <- function(x, n = 6L, ...) { } tail_from_batches <- function(batches, n) { - assert_is(n, c("numeric", "integer")) - assert_that(length(n) == 1) # Negative n requires knowing nrow(x), which requires a scan itself - assert_that(n >= 0) + check_number_decimal(n, min = 0) if (!is.integer(n)) { n <- floor(n) } diff --git a/r/R/dplyr-eval.R b/r/R/dplyr-eval.R index 1282f171878d..f85b63fc2370 100644 --- a/r/R/dplyr-eval.R +++ b/r/R/dplyr-eval.R @@ -47,7 +47,7 @@ arrow_eval <- function(expr, mask) { stop(e) } - # 2. Error is from assert_that: raise as validation_error + # 2. Error is from assert_is: raise as validation_error if (inherits(e, "assertError")) { validation_error(msg, call = expr) } @@ -171,7 +171,7 @@ i18ize_error_messages <- function() { #' suggestion, when the error is ultimately raised by `try_error_dplyr()`, #' `Call collect() first to pull data into R` won't be the only suggestion. #' -#' You can still use `match.arg()` and `assert_that()` for simple input +#' You can still use `match.arg()` and `assert_is()` for simple input #' validation inside of the function bindings. `arrow_eval()` will catch their #' errors and re-raise them as `validation_error`. #' diff --git a/r/R/dplyr-funcs-string.R b/r/R/dplyr-funcs-string.R index 158bae2db87c..1cfe350dd2cf 100644 --- a/r/R/dplyr-funcs-string.R +++ b/r/R/dplyr-funcs-string.R @@ -292,7 +292,7 @@ register_bindings_string_regex <- function() { "stringr::str_count", function(string, pattern) { opts <- get_stringr_pattern_options(enquo(pattern)) - if (!is.string(pattern)) { + if (!is_string(pattern)) { arrow_not_supported("`pattern` must be a length 1 character vector; other values") } arrow_fun <- ifelse(opts$fixed, "count_substring", "count_substring_regex") @@ -422,7 +422,7 @@ register_bindings_string_regex <- function() { }) register_binding("base::strsplit", function(x, split, fixed = FALSE, perl = FALSE, useBytes = FALSE) { - assert_that(is.string(split)) + check_string(split) arrow_fun <- ifelse(fixed, "split_pattern", "split_pattern_regex") # warn when the user specifies both fixed = TRUE and perl = TRUE, for @@ -620,9 +620,11 @@ register_bindings_string_other <- function() { ) register_binding("stringr::str_pad", function(string, width, side = c("left", "right", "both"), pad = " ") { - assert_that(is_integerish(width)) + if (!is_integerish(width)) { + abort("width must be whole numbers.") + } side <- match.arg(side) - assert_that(is.string(pad)) + check_string(pad) if (side == "left") { pad_func <- "utf8_lpad" diff --git a/r/R/dplyr-funcs-type.R b/r/R/dplyr-funcs-type.R index 0c8864ac6c77..e589900bc766 100644 --- a/r/R/dplyr-funcs-type.R +++ b/r/R/dplyr-funcs-type.R @@ -78,7 +78,7 @@ register_bindings_type_cast <- function() { }) register_binding("methods::is", function(object, class2) { - if (is.string(class2)) { + if (is_string(class2)) { switch( class2, # for R data types, pass off to is.*() functions @@ -245,26 +245,26 @@ register_bindings_type_inspect <- function() { # rlang::is_* type functions register_binding("rlang::is_character", function(x, n = NULL) { - assert_that(is.null(n)) + check_null(n) call_binding("is.character", x) }) register_binding("rlang::is_double", function(x, n = NULL, finite = NULL) { - assert_that(is.null(n)) + check_null(n) if (!is.null(finite)) { arrow_not_supported("`finite` argument") } call_binding("is.double", x) }) register_binding("rlang::is_integer", function(x, n = NULL) { - assert_that(is.null(n)) + check_null(n) call_binding("is.integer", x) }) register_binding("rlang::is_list", function(x, n = NULL) { - assert_that(is.null(n)) + check_null(n) call_binding("is.list", x) }) register_binding("rlang::is_logical", function(x, n = NULL) { - assert_that(is.null(n)) + check_null(n) call_binding("is.logical", x) }) } @@ -277,8 +277,7 @@ register_bindings_type_elementwise <- function() { register_binding("base::is.nan", function(x) { if ( is.double(x) || - (inherits(x, "Expression") && - x$type_id() %in% TYPES_WITH_NAN) + (inherits(x, "Expression") && x$type_id() %in% TYPES_WITH_NAN) ) { # TODO: if an option is added to the is_nan kernel to treat NA as NaN, # use that to simplify the code here (ARROW-13366) @@ -312,11 +311,8 @@ register_bindings_type_format <- function() { if (!inherits(x, "Expression")) { return(format(x, ...)) } - - if ( - inherits(x, "Expression") && - x$type_id() %in% Type[c("TIMESTAMP", "DATE32", "DATE64")] - ) { + accepted_types <- Type[c("TIMESTAMP", "DATE32", "DATE64")] + if (inherits(x, "Expression") && x$type_id() %in% accepted_types) { binding_format_datetime(x, ...) } else { cast(x, string()) diff --git a/r/R/dplyr-summarize.R b/r/R/dplyr-summarize.R index 7c2a44eec3d5..2e63373ef5d8 100644 --- a/r/R/dplyr-summarize.R +++ b/r/R/dplyr-summarize.R @@ -146,7 +146,7 @@ do_arrow_summarize <- function(.data, ..., .groups = NULL) { # But we don't support anything that returns multiple rows now .groups <- "drop_last" } else { - assert_that(is.string(.groups)) + check_string(.groups) } if (.groups == "drop_last") { out$group_by_vars <- head(.data$group_by_vars, -1) diff --git a/r/R/dplyr.R b/r/R/dplyr.R index 097a48489abc..2c68c7cca9d8 100644 --- a/r/R/dplyr.R +++ b/r/R/dplyr.R @@ -230,8 +230,7 @@ as.data.frame.arrow_dplyr_query <- function(x, row.names = NULL, optional = FALS #' @export head.arrow_dplyr_query <- function(x, n = 6L, ...) { - assert_is(n, c("numeric", "integer")) - assert_that(length(n) == 1) + check_number_decimal(n) if (!is.integer(n)) { n <- floor(n) } @@ -241,8 +240,7 @@ head.arrow_dplyr_query <- function(x, n = 6L, ...) { #' @export tail.arrow_dplyr_query <- function(x, n = 6L, ...) { - assert_is(n, c("numeric", "integer")) - assert_that(length(n) == 1) + check_number_decimal(n) if (!is.integer(n)) { n <- floor(n) } diff --git a/r/R/expression.R b/r/R/expression.R index 78272323c709..3981b43a8971 100644 --- a/r/R/expression.R +++ b/r/R/expression.R @@ -51,11 +51,15 @@ Expression <- R6Class( # schemas in Expression objects (ARROW-13186) schema = NULL, type = function(schema = self$schema) { - assert_that(!is.null(schema)) + if (is.null(schema)) { + stop("schema cannot be NULL.", call. = FALSE) + } compute___expr__type(self, schema) }, type_id = function(schema = self$schema) { - assert_that(!is.null(schema)) + if (is.null(schema)) { + stop("schema cannot be NULL", call. = FALSE) + } compute___expr__type_id(self, schema) }, is_field_ref = function() { @@ -75,7 +79,7 @@ Expression <- R6Class( ) ) Expression$create <- function(function_name, ..., args = list(...), options = empty_named_list()) { - assert_that(is.string(function_name)) + check_string(function_name) # Make sure all inputs are Expressions args <- lapply(args, function(x) { if (!inherits(x, "Expression")) { @@ -99,7 +103,7 @@ Expression$create <- function(function_name, ..., args = list(...), options = em #' @export `$.Expression` <- function(x, name, ...) { - assert_that(is.string(name)) + check_string(name) if (name %in% ls(x)) { get(name, x) } else { @@ -111,7 +115,7 @@ get_nested_field <- function(expr, name) { if (expr$is_field_ref()) { # Make a nested field ref # TODO(#33756): integer (positional) field refs are supported in C++ - assert_that(is.string(name)) + check_string(name) out <- compute___expr__nested_field_ref(expr, name) } else { # Use the struct_field kernel if expr is a struct: @@ -149,7 +153,7 @@ get_nested_field <- function(expr, name) { Expression$field_ref <- function(name) { # TODO(#33756): allow construction of field ref from integer - assert_that(is.string(name)) + check_string(name) compute___expr__field_ref(name) } Expression$scalar <- function(x) { diff --git a/r/R/extension.R b/r/R/extension.R index 1fe073d7401f..cc9eb518702a 100644 --- a/r/R/extension.R +++ b/r/R/extension.R @@ -227,11 +227,13 @@ ExtensionType$new <- function(xp) { } ExtensionType$create <- function(storage_type, extension_name, extension_metadata = raw(), type_class = ExtensionType) { - if (is.string(extension_metadata)) { + if (is_string(extension_metadata)) { extension_metadata <- charToRaw(enc2utf8(extension_metadata)) } - - assert_that(is.string(extension_name), is.raw(extension_metadata)) + check_string(extension_name) + if (!is.raw(extension_metadata)) { + stop("`extension_metadata` must be raw.", call. = FALSE) + } assert_is(storage_type, "DataType") assert_is(type_class, "R6ClassGenerator") diff --git a/r/R/feather.R b/r/R/feather.R index 23d5e4ed20c0..4c33c138fb12 100644 --- a/r/R/feather.R +++ b/r/R/feather.R @@ -76,7 +76,7 @@ write_feather <- function( ) { # Handle and validate options before touching data version <- as.integer(version) - assert_that(version %in% 1:2) + check_number_whole(version, min = 1, max = 2) if (isTRUE(compression)) { compression <- "default" @@ -88,7 +88,7 @@ write_feather <- function( # TODO(ARROW-17221): if (missing(compression)), we could detect_compression(sink) here compression <- match.arg(compression) chunk_size <- as.integer(chunk_size) - assert_that(chunk_size > 0) + check_number_whole(chunk_size, min = 1) if (compression == "default") { if (version == 2 && codec_is_available("lz4")) { compression <- "lz4" diff --git a/r/R/field.R b/r/R/field.R index f1a9c8a2eac1..ec0e7d6c4a25 100644 --- a/r/R/field.R +++ b/r/R/field.R @@ -84,7 +84,7 @@ Field <- R6Class( ) ) Field$create <- function(name, type, metadata = NULL, nullable = TRUE) { - assert_that(inherits(name, "character"), length(name) == 1L) + check_string(name) type <- as_type(type, name) f <- Field__initialize(enc2utf8(name), type, nullable) if (!is.null(metadata)) { @@ -113,7 +113,10 @@ field <- Field$create .fields <- function(.list, nullable = TRUE) { if (length(.list)) { - assert_that(!is.null(nms <- names(.list))) + nms <- names(.list) + if (is.null(nms)) { + stop(".list must be named.", call. = FALSE) + } map2(nms, .list, field) } else { list() diff --git a/r/R/filesystem.R b/r/R/filesystem.R index abbc9cd6c5bf..f7c80b092a77 100644 --- a/r/R/filesystem.R +++ b/r/R/filesystem.R @@ -367,7 +367,7 @@ FileSystem <- R6Class( ) ) FileSystem$from_uri <- function(uri) { - assert_that(is.string(uri)) + check_string(uri) fs___FileSystemFromUri(uri) } @@ -377,7 +377,9 @@ get_paths_and_filesystem <- function(x, filesystem = NULL) { if (inherits(x, "SubTreeFileSystem")) { return(list(fs = x$base_fs, path = x$base_path)) } - assert_that(is.character(x)) + if (!is.character(x)) { + stop("x must be a character vector.", call. = FALSE) + } are_urls <- are_urls(x) if (any(are_urls)) { if (!all(are_urls)) { @@ -412,12 +414,14 @@ get_paths_and_filesystem <- function(x, filesystem = NULL) { # variant of the above function that asserts that x is either a scalar string # or a SubTreeFileSystem get_path_and_filesystem <- function(x, filesystem = NULL) { - assert_that(is.string(x) || inherits(x, "SubTreeFileSystem")) + if (!is_string(x) && !inherits(x, "SubTreeFileSystem")) { + stop("x must be a string or SubTreeFileSystem.") + } get_paths_and_filesystem(x, filesystem) } -is_url <- function(x) is.string(x) && grepl("://", x) -is_http_url <- function(x) is_url(x) && grepl("^http", x) +is_url <- function(x) is_string(x) && grepl("://", x) +is_http_url <- function(x) is_url(x) && startsWith(x, "http") are_urls <- function(x) if (!is.character(x)) FALSE else grepl("://", x) #' @usage NULL @@ -538,7 +542,7 @@ default_s3_options <- list( #' #' @export s3_bucket <- function(bucket, ...) { - assert_that(is.string(bucket)) + check_string(bucket) args <- list2(...) # If user specifies args, they must specify region as arg, env var, or config @@ -573,7 +577,7 @@ s3_bucket <- function(bucket, ...) { #' bucket <- gs_bucket("arrow-datasets") #' @export gs_bucket <- function(bucket, ...) { - assert_that(is.string(bucket)) + check_string(bucket) args <- list2(...) fs <- exec(GcsFileSystem$create, !!!args) @@ -755,7 +759,7 @@ AzureFileSystem$create <- function(account_name, ...) { #' ) #' @export az_container <- function(container_path, ...) { - assert_that(is.string(container_path)) + check_string(container_path) args <- list2(...) fs <- exec(AzureFileSystem$create, !!!args) @@ -798,7 +802,7 @@ SubTreeFileSystem$create <- function(base_path, base_fs = NULL) { #' @export `$.SubTreeFileSystem` <- function(x, name, ...) { # This is to allow delegating methods/properties to the base_fs - assert_that(is.string(name)) + check_string(name) if (name %in% ls(envir = x)) { get(name, x) } else if (name %in% ls(envir = x$base_fs)) { diff --git a/r/R/io.R b/r/R/io.R index 06bd47c0a882..66919636abd8 100644 --- a/r/R/io.R +++ b/r/R/io.R @@ -246,7 +246,7 @@ make_readable_file <- function(file, mmap = TRUE, random_access = TRUE) { # file names with trailing slashes, so we need to remove it here. path <- sub("/$", "", file$base_path) file <- filesystem$OpenInputFile(path) - } else if (is.string(file)) { + } else if (is_string(file)) { # if this is a HTTP URL, we need a local copy to pass to FileSystem$from_uri if (random_access && is_http_url(file)) { tf <- tempfile() @@ -303,7 +303,7 @@ make_output_stream <- function(x) { fs_and_path <- FileSystem$from_uri(x) fs_and_path$fs$OpenOutputStream(fs_and_path$path) } else { - assert_that(is.string(x)) + check_string(x) FileOutputStream$create(x) } } @@ -312,7 +312,7 @@ detect_compression <- function(path) { if (inherits(path, "SubTreeFileSystem")) { path <- path$base_path } - if (!is.string(path)) { + if (!is_string(path)) { return("uncompressed") } diff --git a/r/R/parquet.R b/r/R/parquet.R index 6415e36b03ce..c5d47f7765f3 100644 --- a/r/R/parquet.R +++ b/r/R/parquet.R @@ -251,7 +251,7 @@ make_valid_parquet_version <- function(version, valid_versions = valid_parquet_v version <- format(version, nsmall = 1) } - if (!is.string(version)) { + if (!is_string(version)) { stop( "`version` must be one of ", oxford_paste(names(valid_versions), "or"), @@ -313,7 +313,7 @@ ParquetWriterPropertiesBuilder <- R6Class( }, set_compression = function(column_names, compression) { compression <- compression_from_name(compression) - assert_that(is.integer(compression)) + assert_is(compression, "integer") private$.set( column_names, compression, @@ -330,7 +330,7 @@ ParquetWriterPropertiesBuilder <- R6Class( ) }, set_dictionary = function(column_names, use_dictionary) { - assert_that(is.logical(use_dictionary)) + check_logical(use_dictionary) private$.set( column_names, use_dictionary, @@ -338,7 +338,7 @@ ParquetWriterPropertiesBuilder <- R6Class( ) }, set_write_statistics = function(column_names, write_statistics) { - assert_that(is.logical(write_statistics)) + check_logical(write_statistics) private$.set( column_names, write_statistics, diff --git a/r/R/query-engine.R b/r/R/query-engine.R index f1cbebb72a89..6c823879bd5f 100644 --- a/r/R/query-engine.R +++ b/r/R/query-engine.R @@ -372,7 +372,7 @@ head.ExecPlanReader <- function(x, n = 6L, ...) { } do_exec_plan_substrait <- function(substrait_plan) { - if (is.string(substrait_plan)) { + if (is_string(substrait_plan)) { substrait_plan <- substrait__internal__SubstraitFromJSON(substrait_plan) } else if (is.raw(substrait_plan)) { substrait_plan <- buffer(substrait_plan) diff --git a/r/R/record-batch-reader.R b/r/R/record-batch-reader.R index d979a3f9fe92..ad6b3c612fcd 100644 --- a/r/R/record-batch-reader.R +++ b/r/R/record-batch-reader.R @@ -134,10 +134,8 @@ as.data.frame.RecordBatchReader <- function(x, row.names = NULL, optional = FALS #' @export head.RecordBatchReader <- function(x, n = 6L, ...) { - assert_is(n, c("numeric", "integer")) - assert_that(length(n) == 1) # Negative n requires knowing nrow(x), which requires consuming the whole RBR - assert_that(n >= 0) + check_number_decimal(n, min = 0) if (!is.integer(n)) { n <- floor(n) } @@ -251,7 +249,7 @@ as_record_batch_reader.Dataset <- function(x, ...) { #' @rdname as_record_batch_reader #' @export as_record_batch_reader.function <- function(x, ..., schema) { - assert_that(inherits(schema, "Schema")) + assert_is(schema, "Schema") RecordBatchReader__from_function(x, schema) } diff --git a/r/R/record-batch-writer.R b/r/R/record-batch-writer.R index eb96592a0f5b..49766b7a6000 100644 --- a/r/R/record-batch-writer.R +++ b/r/R/record-batch-writer.R @@ -118,7 +118,7 @@ RecordBatchWriter <- R6Class( #' @export RecordBatchStreamWriter <- R6Class("RecordBatchStreamWriter", inherit = RecordBatchWriter) RecordBatchStreamWriter$create <- function(sink, schema, use_legacy_format = NULL, metadata_version = NULL) { - if (is.string(sink)) { + if (is_string(sink)) { stop( "RecordBatchStreamWriter$create() requires an Arrow InputStream. ", "Try providing FileOutputStream$create(", @@ -144,7 +144,7 @@ RecordBatchStreamWriter$create <- function(sink, schema, use_legacy_format = NUL #' @export RecordBatchFileWriter <- R6Class("RecordBatchFileWriter", inherit = RecordBatchStreamWriter) RecordBatchFileWriter$create <- function(sink, schema, use_legacy_format = NULL, metadata_version = NULL) { - if (is.string(sink)) { + if (is_string(sink)) { stop( "RecordBatchFileWriter$create() requires an Arrow InputStream. ", "Try providing FileOutputStream$create(", diff --git a/r/R/record-batch.R b/r/R/record-batch.R index f175b70ac9d9..c9b88768a1d8 100644 --- a/r/R/record-batch.R +++ b/r/R/record-batch.R @@ -91,7 +91,7 @@ RecordBatch <- R6Class( inherits(other, "RecordBatch") && RecordBatch__Equals(self, other, isTRUE(check_metadata)) }, GetColumnByName = function(name) { - assert_that(is.string(name)) + check_string(name) RecordBatch__GetColumnByName(self, name) }, SelectColumns = function(indices) RecordBatch__SelectColumns(self, indices), @@ -119,7 +119,9 @@ RecordBatch <- R6Class( }, cast = function(target_schema, safe = TRUE, ..., options = cast_options(safe, ...)) { assert_is(target_schema, "Schema") - assert_that(identical(self$schema$names, target_schema$names), msg = "incompatible schemas") + if (!identical(self$schema$names, target_schema$names)) { + stop("incompatible schemas", call. = FALSE) + } RecordBatch__cast(self, target_schema, options) }, export_to_c = function(array_ptr, schema_ptr) { diff --git a/r/R/schema.R b/r/R/schema.R index a02753fac55a..7307f5b8d9f9 100644 --- a/r/R/schema.R +++ b/r/R/schema.R @@ -227,7 +227,7 @@ prepare_key_value_metadata <- function(metadata) { # Alternative to Schema__ToString that doesn't print metadata print_schema_fields <- function(s, truncate = FALSE, max_fields = 20L) { - assert_that(max_fields > 0) + check_number_whole(max_fields, min = 1) num_fields <- length(s$fields) if (truncate && num_fields > max_fields) { fields_out <- paste(map_chr(s$fields[seq_len(max_fields)], ~ .$ToString()), collapse = "\n") @@ -318,7 +318,9 @@ length.Schema <- function(x) x$num_fields #' @export `[[<-.Schema` <- function(x, i, value) { - assert_that(length(i) == 1) + if (length(i) != 1) { + stop("`i` must have length 1.", call. = FALSE) + } if (is.character(i)) { field_names <- names(x) if (anyDuplicated(field_names)) { @@ -333,7 +335,7 @@ length.Schema <- function(x) x$num_fields # No match means we're adding to the end i <- match(i, field_names, nomatch = length(field_names) + 1L) } else { - assert_that(is.numeric(i), !is.na(i), i > 0) + check_number_whole(i, min = 1, allow_na = FALSE) # If i is numeric and we have a type, # we need to grab the existing field name for the new one if (!is.null(value) && !inherits(value, "Field")) { @@ -384,7 +386,7 @@ length.Schema <- function(x) x$num_fields #' @export `$.Schema` <- function(x, name, ...) { - assert_that(is.string(name)) + check_string(name) if (name %in% ls(x)) { get(name, x) } else { diff --git a/r/R/table.R b/r/R/table.R index 4e8662e6e719..a25a6724014a 100644 --- a/r/R/table.R +++ b/r/R/table.R @@ -85,8 +85,7 @@ Table <- R6Class( nbytes = function() Table__ReferencedBufferSize(self), RenameColumns = function(value) Table__RenameColumns(self, value), GetColumnByName = function(name) { - assert_is(name, "character") - assert_that(length(name) == 1) + check_string(name) Table__GetColumnByName(self, name) }, RemoveColumn = function(i) Table__RemoveColumn(self, i), @@ -102,7 +101,9 @@ Table <- R6Class( }, cast = function(target_schema, safe = TRUE, ..., options = cast_options(safe, ...)) { assert_is(target_schema, "Schema") - assert_that(identical(self$schema$names, target_schema$names), msg = "incompatible schemas") + if (!identical(self$schema$names, target_schema$names)) { + stop("Incompatible schemas", call. = FALSE) + } Table__cast(self, target_schema, options) }, SelectColumns = function(indices) Table__SelectColumns(self, indices), diff --git a/r/R/type.R b/r/R/type.R index 14a4c8f1d2b6..01440623e10f 100644 --- a/r/R/type.R +++ b/r/R/type.R @@ -608,7 +608,7 @@ timestamp <- function(unit = c("s", "ms", "us", "ns"), timezone = "") { unit <- match.arg(unit) } unit <- make_valid_time_unit(unit, c(valid_time64_units, valid_time32_units)) - assert_that(is.string(timezone)) + check_string(timezone) Timestamp__initialize(unit, timezone) } @@ -775,16 +775,14 @@ as_type <- function(type, name = "type") { if (identical(type, double())) { type <- float64() } - if (!inherits(type, "DataType")) { - stop(name, " must be a DataType, not ", class(type), call. = FALSE) - } + assert_is(type, "DataType", arg = name, call = NULL) type } canonical_type_str <- function(type_str) { # canonicalizes data type strings, converting data type function names and # aliases to match the strings returned by DataType$ToString() - assert_that(is.string(type_str)) + check_string(type_str) if (grepl("[([<]", type_str)) { stop("Cannot interpret string representations of data types that have parameters", call. = FALSE) } diff --git a/r/R/udf.R b/r/R/udf.R index ce7a911e0d97..24decdbf734d 100644 --- a/r/R/udf.R +++ b/r/R/udf.R @@ -72,7 +72,7 @@ #' head() #' register_scalar_function <- function(name, fun, in_type, out_type, auto_convert = FALSE) { - assert_that(is.string(name)) + check_string(name) scalar_function <- arrow_scalar_function( fun, @@ -99,7 +99,9 @@ register_scalar_function <- function(name, fun, in_type, out_type, auto_convert } arrow_scalar_function <- function(fun, in_type, out_type, auto_convert = FALSE) { - assert_that(is.function(fun)) + if (!is.function(fun)) { + stop("`fun` must be a function.") + } # Create a small wrapper function that is easier to call from C++. # TODO(ARROW-17148): This wrapper could be implemented in C/C++ to diff --git a/r/R/util.R b/r/R/util.R index dc48c5fb8ff8..899f565bd4a9 100644 --- a/r/R/util.R +++ b/r/R/util.R @@ -30,14 +30,135 @@ oxford_paste <- function(x, conjunction = "and", quote = TRUE, quote_symbol = '" } } -assert_is <- function(object, class) { - msg <- paste(substitute(object), "must be a", oxford_paste(class, "or")) - assert_that(inherits(object, class), msg = msg) +check_null <- function(x, msg = NULL, call = rlang::caller_env()) { + if (!is.null(x)) { + if (is.null(msg)) { + rlang::stop_input_type( + x, + what = "NULL", + call = call + ) + } else { + abort(msg, call = call) + } + } +} + +# copied from https://github.com/r-lib/rlang/blob/main/R/standalone-types-check.R +# they are not licensed. + +check_character <- function( + x, + ..., + allow_na = TRUE, + allow_null = FALSE, + arg = caller_arg(x), + call = caller_env() +) { + if (!missing(x)) { + if (is_character(x)) { + if (!allow_na && any(is.na(x))) { + abort( + sprintf("`%s` can't contain NA values.", arg), + arg = arg, + call = call + ) + } + + 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 + ) +} + +check_logical <- function( + x, + ..., + allow_na = TRUE, + allow_null = FALSE, + arg = caller_arg(x), + call = caller_env() +) { + if (!missing(x)) { + if (is_logical(x)) { + if (!allow_na && any(is.na(x))) { + abort( + sprintf("`%s` can't contain NA values.", arg), + arg = arg, + call = call + ) + } + return(invisible(NULL)) + } + if (allow_null && is_null(x)) { + return(invisible(NULL)) + } + } + + stop_input_type( + x, + "a logical vector", + ..., + allow_na = FALSE, + allow_null = allow_null, + arg = arg, + call = call + ) +} + +assert_is <- function( + object, + class, + call = caller_env(), + arg = caller_arg(object), + allow_null = FALSE +) { + if (is.null(object) && allow_null) { + return(invisible(NULL)) + } + + if (!inherits(object, class)) { + # recode for friendlier error message + friendly_types <- c( + "factor" = "a factor", + "Array" = "an Array", + "RandomAccessFile" = "a RandomAccessFile", + "DataType" = "a DataType", + "data.frame" = "a data frame" + ) + class <- vctrs::vec_replace_values( + class, + from = names(friendly_types), + to = unname(friendly_types) + ) + rlang::stop_input_type( + x = object, + what = class, + call = call, + arg = arg, + allow_null = allow_null, + class = "assertError" + ) + } } assert_is_list_of <- function(object, class) { msg <- paste(substitute(object), "must be a list of", oxford_paste(class, "or")) - assert_that(is_list_of(object, class), msg = msg) + if (!is_list_of(object, class)) { + stop(msg, call. = FALSE) + } } is_list_of <- function(object, class) { diff --git a/r/extra-tests/test-read-files.R b/r/extra-tests/test-read-files.R index ced366d2f571..8e25451286cf 100644 --- a/r/extra-tests/test-read-files.R +++ b/r/extra-tests/test-read-files.R @@ -38,10 +38,7 @@ pq_file <- "files/ex_data.parquet" test_that("Can read the file (parquet)", { # We can read with no error, we assert metadata below - expect_error( - df <- read_parquet(pq_file), - NA - ) + expect_no_error(df <- read_parquet(pq_file)) }) ### Parquet @@ -83,10 +80,7 @@ for (comp in c("lz4", "uncompressed", "zstd")) { test_that(paste0("Can read the file (feather ", comp, ")"), { # We can read with no error, we assert metadata below - expect_error( - df <- read_feather(feather_file), - NA - ) + expect_no_error(df <- read_feather(feather_file)) }) test_that(paste0("Can see the metadata (feather ", comp, ")"), { diff --git a/r/man/arrow_not_supported.Rd b/r/man/arrow_not_supported.Rd index be6a001fa1fa..bbcc82e4f4da 100644 --- a/r/man/arrow_not_supported.Rd +++ b/r/man/arrow_not_supported.Rd @@ -49,7 +49,7 @@ data into R. If you have an \code{arrow_not_supported()} error with a \code{>} suggestion, when the error is ultimately raised by \code{try_error_dplyr()}, \verb{Call collect() first to pull data into R} won't be the only suggestion. -You can still use \code{match.arg()} and \code{assert_that()} for simple input +You can still use \code{match.arg()} and \code{assert_is()} for simple input validation inside of the function bindings. \code{arrow_eval()} will catch their errors and re-raise them as \code{validation_error}. } diff --git a/r/tests/testthat/_snaps/csv.md b/r/tests/testthat/_snaps/csv.md new file mode 100644 index 000000000000..4f50baab4443 --- /dev/null +++ b/r/tests/testthat/_snaps/csv.md @@ -0,0 +1,93 @@ +# Writing a CSV errors when unsupported (yet) readr args are used + + Code + write_csv_arrow(tbl, csv_file, append = FALSE) + Condition + Error in `write_csv_arrow()`: + ! Arguments not yet supported in Arrow + * `...` must be empty. + x Problematic argument: + * append = FALSE + +--- + + Code + write_csv_arrow(tbl, csv_file, quote = "all") + Condition + Error in `write_csv_arrow()`: + ! Arguments not yet supported in Arrow + * `...` must be empty. + x Problematic argument: + * quote = "all" + +--- + + Code + write_csv_arrow(tbl, csv_file, escape = "double") + Condition + Error in `write_csv_arrow()`: + ! Arguments not yet supported in Arrow + * `...` must be empty. + x Problematic argument: + * escape = "double" + +--- + + Code + write_csv_arrow(tbl, csv_file, eol = "\n") + Condition + Error in `write_csv_arrow()`: + ! Arguments not yet supported in Arrow + * `...` must be empty. + x Problematic argument: + * eol = "\n" + +--- + + Code + write_csv_arrow(tbl, csv_file, num_threads = 8) + Condition + Error in `write_csv_arrow()`: + ! Arguments not yet supported in Arrow + * `...` must be empty. + x Problematic argument: + * num_threads = 8 + +--- + + Code + write_csv_arrow(tbl, csv_file, progress = FALSE) + Condition + Error in `write_csv_arrow()`: + ! Arguments not yet supported in Arrow + * `...` must be empty. + x Problematic argument: + * progress = FALSE + +--- + + Code + write_csv_arrow(tbl, csv_file, append = FALSE, eol = "\n") + Condition + Error in `write_csv_arrow()`: + ! Arguments not yet supported in Arrow + * `...` must be empty. + x Problematic arguments: + * append = FALSE + * eol = "\n" + +--- + + Code + write_csv_arrow(tbl, csv_file, append = FALSE, quote = "all", escape = "double", + eol = "\n") + Condition + Error in `write_csv_arrow()`: + ! Arguments not yet supported in Arrow + * `...` must be empty. + x Problematic arguments: + * append = FALSE + * quote = "all" + * escape = "double" + * eol = "\n" + diff --git a/r/tests/testthat/_snaps/udf.md b/r/tests/testthat/_snaps/udf.md index 89506a7fbc23..27d17e11db4b 100644 --- a/r/tests/testthat/_snaps/udf.md +++ b/r/tests/testthat/_snaps/udf.md @@ -1,4 +1,4 @@ # arrow_scalar_function() works - fun is not a function + `fun` must be a function. diff --git a/r/tests/testthat/test-Array.R b/r/tests/testthat/test-Array.R index b5233eb30360..89b186647d0c 100644 --- a/r/tests/testthat/test-Array.R +++ b/r/tests/testthat/test-Array.R @@ -97,7 +97,7 @@ test_that("Slice() and RangeEquals()", { y <- x$Slice(10) expect_equal(y$type, int32()) - expect_equal(length(y), 15L) + expect_length(y, 15L) expect_as_vector(y, c(101:110, 201:205)) expect_true(x$RangeEquals(y, 10, 24)) expect_false(x$RangeEquals(y, 9, 23)) @@ -122,7 +122,7 @@ test_that("Slice() and RangeEquals()", { expect_error(x$Slice(10, -1), "Slice 'length' cannot be negative") expect_error(x$Slice(-1, 10), "Slice 'offset' cannot be negative") - expect_warning(x$Slice(10, 15), NA) + expect_no_warning(x$Slice(10, 15)) expect_warning( overslice <- x$Slice(10, 16), "Slice 'length' greater than available length" @@ -130,7 +130,7 @@ test_that("Slice() and RangeEquals()", { expect_equal(length(overslice), 15) expect_warning(z$Slice(2, 10), "Slice 'length' greater than available length") - expect_error(x$RangeEquals(10, 24, 0), 'other must be a "Array"') + expect_error(x$RangeEquals(10, 24, 0), "`other` must be an Array") expect_error(x$RangeEquals(y, NA, 24), "'start_idx' cannot be NA") expect_error(x$RangeEquals(y, 10, NA), "'end_idx' cannot be NA") expect_error(x$RangeEquals(y, 10, 24, NA), "'other_start_idx' cannot be NA") @@ -428,7 +428,7 @@ test_that("integer types cast safety (ARROW-3741, ARROW-5541)", { a <- arrow_array(-(1:10)) for (type in uint_types) { expect_error(a$cast(type), regexp = "Integer value -1 not in range") - expect_error(a$cast(type, safe = FALSE), NA) + expect_no_error(a$cast(type, safe = FALSE)) } }) @@ -452,7 +452,7 @@ test_that("cast to half float works", { test_that("cast input validation", { a <- arrow_array(1:4) - expect_error(a$cast("not a type"), "type must be a DataType, not character") + expect_error(a$cast("not a type"), "`type` must be a DataType, not") }) test_that("arrow_array() supports the type= argument. conversion from INTSXP and int64 to all int types", { @@ -473,7 +473,7 @@ test_that("arrow_array() supports the type= argument. conversion from INTSXP and # Input validation expect_error( arrow_array(5, type = "not a type"), - "type must be a DataType, not character" + "`type` must be a DataType, not" ) }) @@ -869,7 +869,7 @@ test_that("Handling string data with embedded nuls", { array_with_nul <- arrow_array(raws)$cast(utf8()) # no error on conversion, because altrep laziness - v <- expect_error(as.vector(array_with_nul), NA) + expect_no_error(v <- as.vector(array_with_nul)) # attempting materialization -> error @@ -946,12 +946,12 @@ test_that("Array$View() (ARROW-6542)", { expect_equal(length(b), 3L) # Input validation - expect_error(a$View("not a type"), "type must be a DataType, not character") + expect_error(a$View("not a type"), "`type` must be a DataType, not") }) test_that("Array$Validate()", { a <- arrow_array(1:10) - expect_error(a$Validate(), NA) + expect_no_error(a$Validate()) }) test_that("is.Array", { diff --git a/r/tests/testthat/test-RecordBatch.R b/r/tests/testthat/test-RecordBatch.R index dbb21c06568e..a3402eb5c15c 100644 --- a/r/tests/testthat/test-RecordBatch.R +++ b/r/tests/testthat/test-RecordBatch.R @@ -161,9 +161,9 @@ test_that("[[ and $ on RecordBatch", { expect_null(batch$qwerty) expect_null(batch[["asdf"]]) expect_error(batch[[c(4, 3)]]) - expect_error(batch[[NA]], "'i' must be character or numeric, not logical") - expect_error(batch[[NULL]], "'i' must be character or numeric, not NULL") - expect_error(batch[[c("asdf", "jkl;")]], "name is not a string", fixed = TRUE) + expect_error(batch[[NA]], "`i` must be character or numeric, not `NA`") + expect_error(batch[[NULL]], "`i` must be character or numeric, not `NULL`") + expect_error(batch[[c("asdf", "jkl;")]], "`name` must be a single string", fixed = TRUE) }) test_that("[[<- assignment", { @@ -224,12 +224,12 @@ test_that("[[<- assignment", { expect_as_vector(batch$array, 10:1) # nonsense indexes - expect_error(batch[[NA]] <- letters[10:1], "'i' must be character or numeric, not logical") - expect_error(batch[[NULL]] <- letters[10:1], "'i' must be character or numeric, not NULL") - expect_error(batch[[NA_integer_]] <- letters[10:1], "!is.na(i) is not TRUE", fixed = TRUE) - expect_error(batch[[NA_real_]] <- letters[10:1], "!is.na(i) is not TRUE", fixed = TRUE) - expect_error(batch[[NA_character_]] <- letters[10:1], "!is.na(i) is not TRUE", fixed = TRUE) - expect_error(batch[[c(1, 4)]] <- letters[10:1], "length(i) not equal to 1", fixed = TRUE) + expect_error(batch[[NA]] <- letters[10:1], "`i` must be character or numeric, not `NA`") + expect_error(batch[[NULL]] <- letters[10:1], "`i` must be character or numeric, not `NULL`") + expect_error(batch[[NA_integer_]] <- letters[10:1], "`i` must be", fixed = TRUE) + expect_error(batch[[NA_real_]] <- letters[10:1], "`i` must be", fixed = TRUE) + expect_error(batch[[NA_character_]] <- letters[10:1], "`i` must be", fixed = TRUE) + expect_error(batch[[c(1, 4)]] <- letters[10:1], "`i` must be", fixed = TRUE) }) test_that("head and tail on RecordBatch", { diff --git a/r/tests/testthat/test-Table.R b/r/tests/testthat/test-Table.R index 12ef25975c70..5aed6ca02b5f 100644 --- a/r/tests/testthat/test-Table.R +++ b/r/tests/testthat/test-Table.R @@ -100,9 +100,9 @@ test_that("[, [[, $ for Table", { expect_equal_data_frame(tab[-3], tbl[-3]) expect_error(tab[[c(4, 3)]]) - expect_error(tab[[NA]], "'i' must be character or numeric, not logical") - expect_error(tab[[NULL]], "'i' must be character or numeric, not NULL") - expect_error(tab[[c("asdf", "jkl;")]], "length(name) not equal to 1", fixed = TRUE) + expect_error(tab[[NA]], "`i` must be character or numeric, not `NA`") + expect_error(tab[[NULL]], "`i` must be character or numeric, not `NULL`") + expect_error(tab[[c("asdf", "jkl;")]], "`name` must be", fixed = TRUE) expect_error(tab[-3:3], "Invalid column index") expect_error(tab[1000], "Invalid column index") expect_error(tab[1:1000], "Invalid column index") @@ -167,12 +167,16 @@ test_that("[[<- assignment", { expect_as_vector(tab$chunked, 1:10) # nonsense indexes - expect_error(tab[[NA]] <- letters[10:1], "'i' must be character or numeric, not logical") - expect_error(tab[[NULL]] <- letters[10:1], "'i' must be character or numeric, not NULL") - expect_error(tab[[NA_integer_]] <- letters[10:1], "!is.na(i) is not TRUE", fixed = TRUE) - expect_error(tab[[NA_real_]] <- letters[10:1], "!is.na(i) is not TRUE", fixed = TRUE) - expect_error(tab[[NA_character_]] <- letters[10:1], "!is.na(i) is not TRUE", fixed = TRUE) - expect_error(tab[[c(1, 4)]] <- letters[10:1], "length(i) not equal to 1", fixed = TRUE) + expect_error(tab[[NA]] <- letters[10:1], "`i` must be character or numeric, not `NA`") + expect_error(tab[[NULL]] <- letters[10:1], "`i` must be character or numeric, not `NULL`") + expect_error( + tab[[NA_integer_]] <- letters[10:1], + "`i` must be character or numeric, not an integer `NA`.", + fixed = TRUE + ) + expect_error(tab[[NA_real_]] <- letters[10:1], "`i` must be length 1 and not NA", fixed = TRUE) + expect_error(tab[[NA_character_]] <- letters[10:1], "`i` must be length 1 and not NA", fixed = TRUE) + expect_error(tab[[c(1, 4)]] <- letters[10:1], "`i` must be length 1 and not NA", fixed = TRUE) }) test_that("Table$Slice", { diff --git a/r/tests/testthat/test-arrow.R b/r/tests/testthat/test-arrow.R index c6ae27ac5229..ee3e1b92b479 100644 --- a/r/tests/testthat/test-arrow.R +++ b/r/tests/testthat/test-arrow.R @@ -25,17 +25,19 @@ test_that("Can't $new() an object with anything other than a pointer", { test_that("assert_is", { x <- 42 - expect_true(assert_is(x, "numeric")) - expect_true(assert_is(x, c("numeric", "character"))) - expect_error(assert_is(x, "factor"), 'x must be a "factor"') + expect_no_error(assert_is(x, "numeric")) + expect_no_error(assert_is(x, c("numeric", "character"))) + expect_error(assert_is(x, "factor"), "`x` must be a factor") expect_error( assert_is(x, c("factor", "list")), - 'x must be a "factor" or "list"' + "`x` must be a factor or list" ) expect_error( assert_is(x, c("factor", "character", "list")), - 'x must be a "factor", "character", or "list"' + "`x` must be a factor, character, or list" ) + y <- NULL + expect_no_error(assert_is(y, class = "data.frame", allow_null = TRUE)) }) test_that("arrow gracefully fails to load objects from other sessions (ARROW-10071)", { diff --git a/r/tests/testthat/test-chunked-array.R b/r/tests/testthat/test-chunked-array.R index bcadaa889f90..bc50e3e3337f 100644 --- a/r/tests/testthat/test-chunked-array.R +++ b/r/tests/testthat/test-chunked-array.R @@ -86,7 +86,7 @@ test_that("ChunkedArray", { expect_error(x$Slice(10, -1), "Slice 'length' cannot be negative") expect_error(x$Slice(-1, 10), "Slice 'offset' cannot be negative") - expect_warning(x$Slice(10, 15), NA) + expect_no_warning(x$Slice(10, 15)) expect_warning( overslice <- x$Slice(10, 16), "Slice 'length' greater than available length" @@ -393,12 +393,12 @@ test_that("ChunkedArray$View() (ARROW-6542)", { expect_equal(length(b), 7L) expect_all_true(sapply(b$chunks, function(.x) .x$type == float32())) # Input validation - expect_error(a$View("not a type"), "type must be a DataType, not character") + expect_error(a$View("not a type"), "`type` must be a DataType, not") }) test_that("ChunkedArray$Validate()", { a <- ChunkedArray$create(1:10) - expect_error(a$Validate(), NA) + expect_no_error(a$Validate()) }) test_that("[ ChunkedArray", { @@ -488,7 +488,7 @@ test_that("Handling string data with embedded nuls", { )) chunked_array_with_nul <- ChunkedArray$create(raws)$cast(utf8()) - v <- expect_error(as.vector(chunked_array_with_nul), NA) + expect_no_error(v <- as.vector(chunked_array_with_nul)) expect_error( v[], @@ -500,7 +500,7 @@ test_that("Handling string data with embedded nuls", { ) withr::with_options(list(arrow.skip_nul = TRUE), { - v <- expect_warning(as.vector(chunked_array_with_nul), NA) + expect_no_warning(v <- as.vector(chunked_array_with_nul)) expect_warning( expect_identical(v[3], "man"), "Stripping '\\0' (nul) from character vector", diff --git a/r/tests/testthat/test-compute-aggregate.R b/r/tests/testthat/test-compute-aggregate.R index dd79682361bf..dd69ccff0429 100644 --- a/r/tests/testthat/test-compute-aggregate.R +++ b/r/tests/testthat/test-compute-aggregate.R @@ -20,9 +20,9 @@ test_that("list_compute_functions", { expect_false(all(grepl("min", allfuncs))) justmins <- list_compute_functions("^min") expect_true(length(justmins) > 0) - expect_all_true(grepl("min", justmins)) + expect_match(justmins, "min") no_hash_funcs <- list_compute_functions("^hash") - expect_true(length(no_hash_funcs) == 0) + expect_length(no_hash_funcs, 0) }) test_that("sum.Array", { diff --git a/r/tests/testthat/test-compute-vector.R b/r/tests/testthat/test-compute-vector.R index bfc42b432f3f..9a2d129c2f25 100644 --- a/r/tests/testthat/test-compute-vector.R +++ b/r/tests/testthat/test-compute-vector.R @@ -135,9 +135,8 @@ test_that("call_function validation", { call_function("filter", Array$create(1:4), Array$create(c(TRUE, FALSE, TRUE)), options = list(keep_na = TRUE)), "Arguments for execution of vector kernel function 'array_filter' must all be the same length" ) - expect_error( - call_function("filter", record_batch(a = 1:3), Array$create(c(TRUE, FALSE, TRUE)), options = list(keep_na = TRUE)), - NA + expect_no_error( + call_function("filter", record_batch(a = 1:3), Array$create(c(TRUE, FALSE, TRUE)), options = list(keep_na = TRUE)) ) expect_error( call_function("filter", options = list(keep_na = TRUE)), diff --git a/r/tests/testthat/test-csv.R b/r/tests/testthat/test-csv.R index 8fb11c2a5e31..09ae1ef1706e 100644 --- a/r/tests/testthat/test-csv.R +++ b/r/tests/testthat/test-csv.R @@ -430,7 +430,7 @@ test_that("Write a CSV file with invalid input type", { test_that("Write a CSV file with invalid batch size", { expect_error( write_csv_arrow(tbl_no_dates, csv_file, batch_size = -1), - regexp = "batch_size not greater than 0" + regexp = "`batch_size` must be" ) }) @@ -440,7 +440,7 @@ test_that("Write a CSV with custom NA value", { expect_identical(tbl_out1, tbl_no_dates) csv_contents <- readLines(csv_file) - expect_true(any(grepl("NULL_VALUE", csv_contents))) + expect_match(csv_contents, "NULL_VALUE", all = FALSE) tbl_in1 <- read_csv_arrow(csv_file, na = "NULL_VALUE") expect_identical(tbl_in1, tbl_no_dates) @@ -448,7 +448,7 @@ test_that("Write a CSV with custom NA value", { # Also can use null_value in CsvWriteOptions tbl_out1 <- write_csv_arrow(tbl_no_dates, csv_file, write_options = csv_write_options(null_string = "another_null")) csv_contents <- readLines(csv_file) - expect_true(any(grepl("another_null", csv_contents))) + expect_match(csv_contents, "another_null", all = FALSE) tbl_in1 <- read_csv_arrow(csv_file, na = "another_null") expect_identical(tbl_in1, tbl_no_dates) @@ -458,7 +458,7 @@ test_that("Write a CSV with custom NA value", { expect_true(file.exists(csv_file)) csv_contents <- readLines(csv_file) - expect_true(any(grepl(",,", csv_contents))) + expect_match(csv_contents, ",,", all = FALSE) tbl_in1 <- read_csv_arrow(csv_file) expect_identical(tbl_in1, tbl_no_dates) @@ -490,35 +490,15 @@ test_that("time mapping work as expected (ARROW-13624)", { }) test_that("Writing a CSV errors when unsupported (yet) readr args are used", { - expect_error( - write_csv_arrow(tbl, csv_file, append = FALSE), - "The following argument is not yet supported in Arrow: \"append\"" - ) - expect_error( - write_csv_arrow(tbl, csv_file, quote = "all"), - "The following argument is not yet supported in Arrow: \"quote\"" - ) - expect_error( - write_csv_arrow(tbl, csv_file, escape = "double"), - "The following argument is not yet supported in Arrow: \"escape\"" - ) - expect_error( - write_csv_arrow(tbl, csv_file, eol = "\n"), - "The following argument is not yet supported in Arrow: \"eol\"" - ) - expect_error( - write_csv_arrow(tbl, csv_file, num_threads = 8), - "The following argument is not yet supported in Arrow: \"num_threads\"" - ) - expect_error( - write_csv_arrow(tbl, csv_file, progress = FALSE), - "The following argument is not yet supported in Arrow: \"progress\"" - ) - expect_error( - write_csv_arrow(tbl, csv_file, append = FALSE, eol = "\n"), - "The following arguments are not yet supported in Arrow: \"append\" and \"eol\"" - ) - expect_error( + expect_snapshot(error = TRUE, write_csv_arrow(tbl, csv_file, append = FALSE)) + expect_snapshot(error = TRUE, write_csv_arrow(tbl, csv_file, quote = "all")) + expect_snapshot(error = TRUE, write_csv_arrow(tbl, csv_file, escape = "double")) + expect_snapshot(error = TRUE, write_csv_arrow(tbl, csv_file, eol = "\n")) + expect_snapshot(error = TRUE, write_csv_arrow(tbl, csv_file, num_threads = 8)) + expect_snapshot(error = TRUE, write_csv_arrow(tbl, csv_file, progress = FALSE), ) + expect_snapshot(error = TRUE, write_csv_arrow(tbl, csv_file, append = FALSE, eol = "\n")) + expect_snapshot( + error = TRUE, write_csv_arrow( tbl, csv_file, @@ -526,10 +506,6 @@ test_that("Writing a CSV errors when unsupported (yet) readr args are used", { quote = "all", escape = "double", eol = "\n" - ), - paste( - "The following arguments are not yet supported in Arrow: \"append\",", - "\"quote\", \"escape\", and \"eol\"" ) ) }) diff --git a/r/tests/testthat/test-data-type.R b/r/tests/testthat/test-data-type.R index fa2e5bcd6e8d..a868246f5edd 100644 --- a/r/tests/testthat/test-data-type.R +++ b/r/tests/testthat/test-data-type.R @@ -343,11 +343,11 @@ test_that("timestamp type input validation", { ) expect_error( timestamp(timezone = 1231231), - "timezone is not a string" + "`timezone` must be" ) expect_error( timestamp(timezone = c("not", "a", "timezone")), - "timezone is not a string" + "`timezone` must be" ) }) @@ -469,8 +469,8 @@ test_that("DictionaryType validation", { dictionary(utf8(), int32()), "Dictionary index type should be .*integer, got string" ) - expect_error(dictionary(4, utf8()), 'index_type must be a "DataType"') - expect_error(dictionary(int8(), "strings"), 'value_type must be a "DataType"') + expect_error(dictionary(4, utf8()), '`index_type` must be a DataType') + expect_error(dictionary(int8(), "strings"), '`value_type` must be a DataType') }) test_that("decimal type and validation", { diff --git a/r/tests/testthat/test-dataset-dplyr.R b/r/tests/testthat/test-dataset-dplyr.R index 09ca0ef155b7..6504ace7e863 100644 --- a/r/tests/testthat/test-dataset-dplyr.R +++ b/r/tests/testthat/test-dataset-dplyr.R @@ -275,7 +275,7 @@ test_that("compute()/collect(as_data_frame=FALSE)", { expect_r6_class(tab5, "Table") # mutate() was evaluated - expect_true("negint" %in% names(tab5)) + expect_contains(names(tab5), "negint") }) test_that("head/tail on query on dataset", { diff --git a/r/tests/testthat/test-dataset-write.R b/r/tests/testthat/test-dataset-write.R index 8fed358dc372..1614a92d0d77 100644 --- a/r/tests/testthat/test-dataset-write.R +++ b/r/tests/testthat/test-dataset-write.R @@ -67,7 +67,7 @@ test_that("Writing a dataset: CSV->IPC", { as_data_frame = FALSE ) # It shouldn't be there - expect_false("int" %in% names(first)) + expect_disjoint(names(first), "int") }) test_that("Writing a dataset: Parquet->IPC", { @@ -221,9 +221,7 @@ test_that("Writing a dataset: no format specified", { list.files(dst_dir, pattern = "parquet"), "part-0.parquet" ) - expect_true( - inherits(new_ds$format, "ParquetFileFormat") - ) + expect_s3_class(new_ds$format, "ParquetFileFormat") expect_equal( new_ds |> collect(), example_data @@ -581,9 +579,7 @@ test_that("max_rows_per_file = 0 does not trigger max_rows_per_group adjustment # max_rows_per_file = 0 means "no limit" and should not error dst_dir <- make_temp_dir() - expect_no_error( - write_dataset(df1, dst_dir, max_rows_per_file = 0L) - ) + expect_no_error(write_dataset(df1, dst_dir, max_rows_per_file = 0L)) }) diff --git a/r/tests/testthat/test-dataset.R b/r/tests/testthat/test-dataset.R index a64ea4cc47f6..2c3135b5f3dc 100644 --- a/r/tests/testthat/test-dataset.R +++ b/r/tests/testthat/test-dataset.R @@ -690,16 +690,8 @@ test_that("scalar aggregates with many batches (ARROW-16904)", { ds <- open_dataset(tf) replicate(100, ds |> summarize(min(x)) |> pull()) - expect_true( - all( - replicate(100, ds |> summarize(min(x)) |> pull() |> as.vector()) == 1 - ) - ) - expect_true( - all( - replicate(100, ds |> summarize(max(x)) |> pull() |> as.vector()) == 100 - ) - ) + expect_all_true(replicate(100, ds |> summarize(min(x)) |> pull() |> as.vector()) == 1) + expect_all_true(replicate(100, ds |> summarize(max(x)) |> pull() |> as.vector()) == 100) }) test_that("streaming map_batches into an ExecPlan", { @@ -1446,7 +1438,7 @@ test_that("FileSystemFactoryOptions input validation", { partitioning = "part", factory_options = list(partition_base_dir = 42) ), - "factory_options$partition_base_dir is not a string", + "`factory_options$partition_base_dir` must be a single string", fixed = TRUE ) expect_error( diff --git a/r/tests/testthat/test-dplyr-filter.R b/r/tests/testthat/test-dplyr-filter.R index ad69b26be798..a5416d1ad476 100644 --- a/r/tests/testthat/test-dplyr-filter.R +++ b/r/tests/testthat/test-dplyr-filter.R @@ -287,27 +287,25 @@ test_that("filter environment scope", { }) test_that("Filtering on a column that doesn't exist errors correctly", { - # expect_warning(., NA) because the usual behavior when it hits a filter + # expect_no_warning() because the usual behavior when it hits a filter # that it can't evaluate is to raise a warning, collect() to R, and retry # the filter. But we want this to error the first time because it's # a user error, not solvable by retrying in R - expect_warning( + expect_no_warning( expect_error( tbl |> record_batch() |> filter(not_a_col == 42) |> collect(), "object 'not_a_col' not found" - ), - NA + ) ) }) test_that("Filtering on a non-existent column errors in the correct language", { with_language("fr", { - expect_warning( + expect_no_warning( expect_error( tbl |> record_batch() |> filter(not_a_col == 42) |> collect(), "objet 'not_a_col' introuvable" - ), - NA + ) ) }) }) diff --git a/r/tests/testthat/test-dplyr-mutate.R b/r/tests/testthat/test-dplyr-mutate.R index 3e116a1012ca..f918b0007165 100644 --- a/r/tests/testthat/test-dplyr-mutate.R +++ b/r/tests/testthat/test-dplyr-mutate.R @@ -514,16 +514,15 @@ test_that("handle bad expressions", { # that need to be forced to fail because they error ambiguously with_language("fr", { - # expect_warning(., NA) because the usual behavior when it hits a filter + # expect_no_warning() because the usual behavior when it hits a filter # that it can't evaluate is to raise a warning, collect() to R, and retry # the filter. But we want this to error the first time because it's # a user error, not solvable by retrying in R - expect_warning( + expect_no_warning( expect_error( Table$create(tbl) |> mutate(newvar = NOTAVAR + 2), "objet 'NOTAVAR' introuvable" - ), - NA + ) ) }) }) diff --git a/r/tests/testthat/test-duckdb.R b/r/tests/testthat/test-duckdb.R index c0e69026488d..8c5611f900e1 100644 --- a/r/tests/testthat/test-duckdb.R +++ b/r/tests/testthat/test-duckdb.R @@ -236,7 +236,7 @@ test_that("Joining, auto-cleanup enabled", { ".int" ) ) - expect_identical(dim(res), c(9L, 14L)) + expect_shape(res, dim = c(9L, 14L)) # clean up cleans up the tables expect_all_true(c(table_one_name, table_two_name) %in% duckdb::duckdb_list_arrow(con)) diff --git a/r/tests/testthat/test-expression.R b/r/tests/testthat/test-expression.R index 2f40c621ec8a..f6a8e5a8be66 100644 --- a/r/tests/testthat/test-expression.R +++ b/r/tests/testthat/test-expression.R @@ -148,7 +148,7 @@ test_that("Nested field ref types", { test_that("Nested field from a non-field-ref (struct_field kernel)", { x <- Expression$scalar(tibble::tibble(a = 1, b = "two")) - expect_true(inherits(x$a, "Expression")) + expect_s3_class(x$a, "Expression") expect_equal(x$a$type(), float64()) expect_error(x$c, "field 'c' not found in struct") }) diff --git a/r/tests/testthat/test-feather.R b/r/tests/testthat/test-feather.R index 188a562fe81b..6a2a4c2fdb14 100644 --- a/r/tests/testthat/test-feather.R +++ b/r/tests/testthat/test-feather.R @@ -209,7 +209,7 @@ test_that("read_feather requires RandomAccessFile and errors nicely otherwise (A skip_if_not_available("gzip") expect_error( read_feather(CompressedInputStream$create(feather_file)), - 'file must be a "RandomAccessFile"' + '`file` must be a RandomAccessFile' ) }) @@ -254,7 +254,7 @@ test_that("read_feather closes connection to file", { write_feather(tib, sink = tf) expect_true(file.exists(tf)) read_feather(tf) - expect_error(file.remove(tf), NA) + expect_no_error(file.remove(tf)) expect_false(file.exists(tf)) }) @@ -333,6 +333,6 @@ test_that("Can read Feather files from a URL", { skip_on_cran() feather_url <- "https://github.com/apache/arrow-testing/raw/master/data/arrow-ipc-stream/integration/1.0.0-littleendian/generated_datetime.arrow_file" # nolint fu <- read_feather(feather_url) - expect_true(tibble::is_tibble(fu)) - expect_identical(dim(fu), c(17L, 15L)) + expect_s3_class(fu, "tbl_df") + expect_shape(fu, dim = c(17L, 15L)) }) diff --git a/r/tests/testthat/test-field.R b/r/tests/testthat/test-field.R index e1972180fcea..46f79985d262 100644 --- a/r/tests/testthat/test-field.R +++ b/r/tests/testthat/test-field.R @@ -33,7 +33,7 @@ test_that("Field with nullable values", { }) test_that("Field validation", { - expect_error(schema(b = 32), "b must be a DataType, not numeric") + expect_error(schema(b = 32), "`b` must be a DataType") }) test_that("Print method for field", { diff --git a/r/tests/testthat/test-install-arrow.R b/r/tests/testthat/test-install-arrow.R index 5415fc27581c..2cfadd99bfc1 100644 --- a/r/tests/testthat/test-install-arrow.R +++ b/r/tests/testthat/test-install-arrow.R @@ -35,6 +35,5 @@ test_that("arrow_repos", { }) test_that("on_rosetta() does not warn", { - # There is no warning - expect_warning(on_rosetta(), NA) + expect_no_warning(on_rosetta()) }) diff --git a/r/tests/testthat/test-memory-pool.R b/r/tests/testthat/test-memory-pool.R index e3adeb675fe3..2b71a606b50c 100644 --- a/r/tests/testthat/test-memory-pool.R +++ b/r/tests/testthat/test-memory-pool.R @@ -20,7 +20,6 @@ test_that("default_memory_pool and its attributes", { # Not integer bc can be >2gb, so we cast to double expect_type(pool$bytes_allocated, "double") expect_type(pool$max_memory, "double") - expect_true(pool$backend_name %in% c("system", "jemalloc", "mimalloc")) - - expect_all_true(supported_memory_backends() %in% c("system", "jemalloc", "mimalloc")) + expect_in(pool$backend_name, c("system", "jemalloc", "mimalloc")) + expect_in(supported_memory_backends(), c("system", "jemalloc", "mimalloc")) }) diff --git a/r/tests/testthat/test-metadata.R b/r/tests/testthat/test-metadata.R index fea45786357e..f7a30ac38c4f 100644 --- a/r/tests/testthat/test-metadata.R +++ b/r/tests/testthat/test-metadata.R @@ -141,13 +141,12 @@ arbitrary\040code\040was\040just\040executed 254 " ) - expect_message( + expect_no_message( expect_warning( as.data.frame(tab), 'Invalid metadata$[["r"]]', fixed = TRUE - ), - NA + ) ) }) @@ -382,9 +381,8 @@ test_that("Row-level metadata (does not) roundtrip in datasets", { ) # however there is *no* warning if we don't select the metadata column - expect_warning( - df_from_ds <- ds |> dplyr::select(int) |> dplyr::collect(), - NA + expect_no_warning( + df_from_ds <- ds |> dplyr::select(int) |> dplyr::collect() ) }) diff --git a/r/tests/testthat/test-parquet.R b/r/tests/testthat/test-parquet.R index faa8d41e23db..7fb7b542ace2 100644 --- a/r/tests/testthat/test-parquet.R +++ b/r/tests/testthat/test-parquet.R @@ -22,8 +22,8 @@ pq_file <- system.file("v0.7.1.parquet", package = "arrow") test_that("reading a known Parquet file to tibble", { skip_if_not_available("snappy") df <- read_parquet(pq_file) - expect_true(tibble::is_tibble(df)) - expect_identical(dim(df), c(10L, 11L)) + expect_s3_class(df, "tbl_df") + expect_shape(df, dim = c(10L, 11L)) # TODO: assert more about the contents }) @@ -35,7 +35,7 @@ test_that("simple int column roundtrip", { df_read <- read_parquet(pq_tmp_file, mmap = FALSE) expect_equal(df, df_read) # Make sure file connection is cleaned up - expect_error(file.remove(pq_tmp_file), NA) + expect_no_error(file.remove(pq_tmp_file)) expect_false(file.exists(pq_tmp_file)) }) @@ -52,7 +52,7 @@ test_that("read_parquet() with raw data", { skip_if_not_available("snappy") test_raw <- readBin(pq_file, what = "raw", n = 5000) df <- read_parquet(test_raw) - expect_identical(dim(df), c(10L, 11L)) + expect_shape(df, dim = c(10L, 11L)) }) test_that("write_parquet() handles various compression= specs", { @@ -85,7 +85,7 @@ test_that("write_parquet() handles various use_dictionary= specs", { ) expect_error( write_parquet(tab, tempfile(), use_dictionary = 12), - "is.logical(use_dictionary) is not TRUE", + "`use_dictionary` must be a logical vector", fixed = TRUE ) }) @@ -312,7 +312,7 @@ test_that("ParquetFileReader raises an error for non-RandomAccessFile source", { skip_if_not_available("gzip") expect_error( ParquetFileReader$create(CompressedInputStream$create(pq_file)), - 'file must be a "RandomAccessFile"' + '`file` must be a RandomAccessFile' ) }) @@ -489,8 +489,8 @@ test_that("Can read Parquet files from a URL", { skip_if_not_available("snappy") parquet_url <- "https://github.com/apache/arrow/raw/64f2cc7986ce672dd1a8cb268d193617a80a1653/r/inst/v0.7.1.parquet" # nolint pu <- read_parquet(parquet_url) - expect_true(tibble::is_tibble(pu)) - expect_identical(dim(pu), c(10L, 11L)) + expect_s3_class(pu, "tbl_df") + expect_shape(pu, dim = c(10L, 11L)) }) test_that("thrift string and container size can be specified when reading Parquet files", { diff --git a/r/tests/testthat/test-python-flight.R b/r/tests/testthat/test-python-flight.R index 2e60957d2ff7..97c665f75c4e 100644 --- a/r/tests/testthat/test-python-flight.R +++ b/r/tests/testthat/test-python-flight.R @@ -34,7 +34,7 @@ if (process_is_running("demo_flight_server")) { expect_true(flight_obj %in% list_flights(client)) expect_error( flight_put(client, Array$create(c(1:3)), path = flight_obj), - regexp = 'data must be a "data.frame", "Table", or "RecordBatch"' + regexp = '`data` must be a data frame, Table, or RecordBatch' ) }) @@ -48,7 +48,7 @@ if (process_is_running("demo_flight_server")) { ) expect_error( flight_put(client, Array$create(c(1:3)), path = flight_obj), - regexp = 'data must be a "data.frame", "Table", or "RecordBatch"' + regexp = '`data` must be a data frame, Table, or RecordBatch' ) }) diff --git a/r/tests/testthat/test-python.R b/r/tests/testthat/test-python.R index 0da8539ddef4..9626ddf21b84 100644 --- a/r/tests/testthat/test-python.R +++ b/r/tests/testthat/test-python.R @@ -30,7 +30,7 @@ test_that("install_pyarrow", { venv <- try(reticulate::virtualenv_create("arrow-test")) # Bail out if virtualenv isn't available skip_if(inherits(venv, "try-error")) - expect_error(install_pyarrow("arrow-test", nightly = TRUE), NA) + expect_no_error(install_pyarrow("arrow-test", nightly = TRUE)) # Set this up for the following tests reticulate::use_virtualenv("arrow-test") }) diff --git a/r/tests/testthat/test-read-record-batch.R b/r/tests/testthat/test-read-record-batch.R index 7f310e8fc91c..ff004201c390 100644 --- a/r/tests/testthat/test-read-record-batch.R +++ b/r/tests/testthat/test-read-record-batch.R @@ -39,7 +39,7 @@ test_that("RecordBatchFileWriter / RecordBatchFileReader roundtrips", { expect_equal(read_feather(tf, as_data_frame = FALSE, mmap = FALSE), tab) # Make sure connections are closed - expect_error(file.remove(tf), NA) + expect_no_error(file.remove(tf)) skip_on_os("windows") # This should pass, we've closed the stream expect_false(file.exists(tf)) }) diff --git a/r/tests/testthat/test-s3-minio.R b/r/tests/testthat/test-s3-minio.R index de1cad547e19..3417cfc682d5 100644 --- a/r/tests/testthat/test-s3-minio.R +++ b/r/tests/testthat/test-s3-minio.R @@ -124,7 +124,7 @@ test_that("Confirm s3_bucket works with endpoint_override", { os <- bucket$OpenOutputStream("bucket-test.csv") write_csv_arrow(example_data, os) os$close() - expect_true("bucket-test.csv" %in% bucket$ls()) + expect_contains(bucket$ls(), "bucket-test.csv") bucket$DeleteFile("bucket-test.csv") }) diff --git a/r/tests/testthat/test-s3.R b/r/tests/testthat/test-s3.R index 7818f1e3d43a..ad8c9727008a 100644 --- a/r/tests/testthat/test-s3.R +++ b/r/tests/testthat/test-s3.R @@ -59,7 +59,7 @@ if (run_these) { test_that("RandomAccessFile$ReadMetadata() works for S3FileSystem", { file <- bucket$OpenInputFile(paste0(now, "/", "test.parquet")) metadata <- file$ReadMetadata() - expect_true(is.list(metadata)) - expect_true("ETag" %in% names(metadata)) + expect_type(metadata, "list") + expect_contains(names(metadata), "ETag") }) } diff --git a/r/tests/testthat/test-schema.R b/r/tests/testthat/test-schema.R index 93fb16c77fbd..3a18edd98158 100644 --- a/r/tests/testthat/test-schema.R +++ b/r/tests/testthat/test-schema.R @@ -131,12 +131,12 @@ test_that("Schema modification", { expect_equal(schm, schema(b = double(), y = int64(), d = int8())) # Error handling - expect_error(schm$c <- 4, "value must be a DataType") - expect_error(schm[[-3]] <- int32(), "i not greater than 0") - expect_error(schm[[0]] <- int32(), "i not greater than 0") - expect_error(schm[[NA_integer_]] <- int32(), "!is.na(i) is not TRUE", fixed = TRUE) - expect_error(schm[[TRUE]] <- int32(), "i is not a numeric or integer vector") - expect_error(schm[[c(2, 4)]] <- int32(), "length(i) not equal to 1", fixed = TRUE) + expect_error(schm$c <- 4, "`value` must be a DataType") + expect_error(schm[[-3]] <- int32(), "`i` must be") + expect_error(schm[[0]] <- int32(), "`i` must be") + expect_error(schm[[NA_integer_]] <- int32(), "`i` must be", fixed = TRUE) + expect_error(schm[[TRUE]] <- int32(), "`i` must be") + expect_error(schm[[c(2, 4)]] <- int32(), "`i` must have length 1", fixed = TRUE) }) test_that("Metadata can be reassigned as a whole", { @@ -326,6 +326,6 @@ test_that("schema print truncation", { expect_error( print_schema_fields(schema(tbl), truncate = TRUE, max_fields = 0), - regexp = "max_fields not greater than 0" + regexp = "must be a whole number larger than or equal to 1" ) }) diff --git a/r/tests/testthat/test-udf.R b/r/tests/testthat/test-udf.R index 2eadd87444b5..156edd0b6de9 100644 --- a/r/tests/testthat/test-udf.R +++ b/r/tests/testthat/test-udf.R @@ -17,7 +17,7 @@ test_that("list_compute_functions() works", { expect_type(list_compute_functions(), "character") - expect_all_false(grepl("^hash_", list_compute_functions())) + expect_no_match(list_compute_functions(), "^hash_") }) test_that("arrow_scalar_function() works", { @@ -95,8 +95,8 @@ test_that("register_scalar_function() adds a compute function to the registry", ) on.exit(unregister_binding("times_32")) - expect_true("times_32" %in% names(asNamespace("arrow")$.cache$functions)) - expect_true("times_32" %in% list_compute_functions()) + expect_contains(names(asNamespace("arrow")$.cache$functions), "times_32") + expect_contains(list_compute_functions(), "times_32") expect_equal( call_function("times_32", Array$create(1L, int32())),