thisisnic commented on code in PR #14705:
URL: https://github.com/apache/arrow/pull/14705#discussion_r1034364566
##########
r/R/dataset-format.R:
##########
@@ -312,7 +326,30 @@ csv_file_format_read_opts <- function(schema = NULL, ...) {
if (!is.null(schema) && is.null(opts[["column_names"]])) {
opts[["column_names"]] <- names(schema)
}
- do.call(CsvReadOptions$create, opts)
+
+ opt_names <- names(opts)
+ arrow_opts <- c(names(formals(CsvReadOptions$create)))
+ readr_opts <- c(names(formals(readr_to_csv_read_options)))
+
+ is_arrow_opt <- !is.na(match(opt_names, arrow_opts))
+ is_readr_opt <- !is.na(match(opt_names, readr_opts))
+
+ check_ambiguous_options(opt_names, arrow_opts, readr_opts)
+
+ if (any(is_readr_opt)) {
+ # Catch cases when the user specifies a mix of Arrow C++ options and
+ # readr-style options
+ if (!all(is_readr_opt)) {
+ abort(c(
+ "Use either Arrow read options or readr read options, not both.",
+ i = sprintf("Passed Arrow options: %s.",
oxford_paste(opt_names[is_arrow_opt])),
+ i = sprintf("Passed readr options: %s.",
oxford_paste(opt_names[is_readr_opt]))
+ ))
Review Comment:
```suggestion
abort(c(
"Additional CSV reading options must be Arrow-style or readr-style,
but not both.",
i = sprintf("Arrow options used: %s.",
oxford_paste(opt_names[is_arrow_opt])),
i = sprintf("readr options used: %s.",
oxford_paste(opt_names[is_readr_opt]))
))
```
How about this slight rephrase which shifts the focus from the person using
the function to the usage of the function itself, and adds in "must" as per
[tidyverse style](https://style.tidyverse.org/error-messages.html)?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
