-
Notifications
You must be signed in to change notification settings - Fork 4.2k
GH-33526: [R] Implement new function open_dataset_csv with signature more closely matching read_csv_arrow #33614
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
Changes from 31 commits
f7c63cf
c7578ac
3e932c0
e057b81
cb2b365
3cbf7a8
cc16968
df25d61
5aa4525
aeb1dbf
1528b95
63f223f
fa50ef0
72f2e7e
677ca8e
e86d84f
d8ae2c9
cf95f94
7f858ea
983362e
40199c0
761afd3
26f9f1b
fb087a9
fdc2658
059fe5e
0468160
7d6b1ce
c85808a
54b529d
785db46
29da6a8
b710eb8
3388076
b3c27a0
e0e8411
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -113,107 +113,104 @@ ParquetFileFormat$create <- function(..., | |
| #' @export | ||
| IpcFileFormat <- R6Class("IpcFileFormat", inherit = FileFormat) | ||
|
|
||
| #' @usage NULL | ||
| #' @format NULL | ||
| #' @rdname FileFormat | ||
| #' CSV dataset file format | ||
| #' | ||
| #' @description | ||
| #' A `CSVFileFormat` is a [FileFormat] subclass which holds information about how to | ||
| #' read and parse the files included in a CSV `Dataset`. | ||
| #' | ||
| #' @section Factory: | ||
| #' `CSVFileFormat$create()` can take options in the form of lists passed through as `parse_options`, | ||
| #' `read_options`, or `convert_options` parameters. Alternatively, readr-style options can be passed | ||
| #' through individually. | ||
| #' | ||
| #' @return A `CsvFileFormat` object | ||
| #' @rdname CsvFileFormat | ||
| #' @name CsvFileFormat | ||
| #' @seealso [FileFormat] | ||
| #' @examplesIf arrow_with_dataset() && tolower(Sys.info()[["sysname"]]) != "windows" | ||
| #' # Set up directory for examples | ||
| #' tf <- tempfile() | ||
| #' dir.create(tf) | ||
| #' on.exit(unlink(tf)) | ||
| #' df <- data.frame(x = c("1", "2", "NULL")) | ||
| #' write.table(df, file.path(tf, "file1.txt"), sep = ",", row.names = FALSE) | ||
| #' | ||
| #' # Create CsvFileFormat object with Arrow-style null_values option | ||
| #' format <- CsvFileFormat$create(convert_options = list(null_values = c("", "NA", "NULL"))) | ||
| #' open_dataset(tf, format = format) | ||
| #' | ||
| #' # Use readr-style options | ||
| #' format <- CsvFileFormat$create(na = c("", "NA", "NULL")) | ||
| #' open_dataset(tf, format = format) | ||
| #' | ||
| #' @export | ||
| CsvFileFormat <- R6Class("CsvFileFormat", inherit = FileFormat) | ||
| CsvFileFormat$create <- function(..., | ||
| opts = csv_file_format_parse_options(...), | ||
| convert_options = csv_file_format_convert_opts(...), | ||
| read_options = csv_file_format_read_opts(...)) { | ||
| check_csv_file_format_args(...) | ||
| # Evaluate opts first to catch any unsupported arguments | ||
| force(opts) | ||
|
|
||
| options <- list(...) | ||
| schema <- options[["schema"]] | ||
| if (!is.null(schema) && !inherits(schema, "Schema")) { | ||
| abort(paste0( | ||
| "`schema` must be an object of class 'Schema' not '", | ||
| class(schema)[1], | ||
| "'." | ||
| )) | ||
| } | ||
|
|
||
| if (!inherits(read_options, "CsvReadOptions")) { | ||
| read_options <- do.call(CsvReadOptions$create, read_options) | ||
| } | ||
| CsvFileFormat$create <- function(...) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. whereby:
suggested:
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good points made; the refactoring I did here is because that bit of the code was overly complex, so in the name of usability, I think it'll make more sense to write actual independent docs for this function with more explicit information about its usage. |
||
| dots <- list(...) | ||
| options <- check_csv_file_format_args(dots) | ||
| check_schema(options[["schema"]], options[["read_options"]]$column_names) | ||
|
|
||
| if (!inherits(convert_options, "CsvConvertOptions")) { | ||
| convert_options <- do.call(CsvConvertOptions$create, convert_options) | ||
| } | ||
|
|
||
| if (!inherits(opts, "CsvParseOptions")) { | ||
| opts <- do.call(CsvParseOptions$create, opts) | ||
| } | ||
|
|
||
| column_names <- read_options$column_names | ||
| schema_names <- names(schema) | ||
| dataset___CsvFileFormat__Make(options$parse_options, options$convert_options, options$read_options) | ||
| } | ||
|
|
||
| if (!is.null(schema) && !identical(schema_names, column_names)) { | ||
| missing_from_schema <- setdiff(column_names, schema_names) | ||
| missing_from_colnames <- setdiff(schema_names, column_names) | ||
| message_colnames <- NULL | ||
| message_schema <- NULL | ||
| message_order <- NULL | ||
| # Check all arguments are valid | ||
| check_csv_file_format_args <- function(args) { | ||
| options <- list( | ||
| parse_options = args$parse_options, | ||
| convert_options = args$convert_options, | ||
| read_options = args$read_options, | ||
| schema = args$schema | ||
| ) | ||
|
|
||
| if (length(missing_from_colnames) > 0) { | ||
| message_colnames <- paste( | ||
| oxford_paste(missing_from_colnames, quote_symbol = "`"), | ||
| "not present in `column_names`" | ||
| ) | ||
| } | ||
| check_unsupported_args(args) | ||
| check_unrecognised_args(args) | ||
|
|
||
| if (length(missing_from_schema) > 0) { | ||
| message_schema <- paste( | ||
| oxford_paste(missing_from_schema, quote_symbol = "`"), | ||
| "not present in `schema`" | ||
| ) | ||
| } | ||
| # Evaluate parse_options first to catch any unsupported arguments | ||
| if (is.null(args$parse_options)) { | ||
| options$parse_options <- do.call(csv_file_format_parse_opts, args) | ||
| } else if (is.list(args$parse_options)) { | ||
| options$parse_options <- do.call(CsvParseOptions$create, args$parse_options) | ||
| } | ||
|
|
||
| if (length(missing_from_schema) == 0 && length(missing_from_colnames) == 0) { | ||
| message_order <- "`column_names` and `schema` field names match but are not in the same order" | ||
| } | ||
| if (is.null(args$convert_options)) { | ||
| options$convert_options <- do.call(csv_file_format_convert_opts, args) | ||
| } else if (is.list(args$convert_options)) { | ||
| options$convert_options <- do.call(CsvConvertOptions$create, args$convert_options) | ||
| } | ||
|
|
||
| abort( | ||
| c( | ||
| "Values in `column_names` must match `schema` field names", | ||
| x = message_order, | ||
| x = message_schema, | ||
| x = message_colnames | ||
| ) | ||
| ) | ||
| if (is.null(args$read_options)) { | ||
| options$read_options <- do.call(csv_file_format_read_opts, args) | ||
| } else if (is.list(args$read_options)) { | ||
| options$read_options <- do.call(CsvReadOptions$create, args$read_options) | ||
| } | ||
|
|
||
| dataset___CsvFileFormat__Make(opts, convert_options, read_options) | ||
| options | ||
| } | ||
|
|
||
| # Check all arguments are valid | ||
| check_csv_file_format_args <- function(...) { | ||
| opts <- list(...) | ||
| check_unsupported_args <- function(args) { | ||
| opt_names <- get_opt_names(args) | ||
|
|
||
| # Filter out arguments meant for CsvConvertOptions/CsvReadOptions | ||
| convert_opts <- c(names(formals(CsvConvertOptions$create))) | ||
| supported_convert_opts <- c(names(formals(CsvConvertOptions$create)), "na") | ||
|
|
||
| read_opts <- c( | ||
| supported_read_opts <- c( | ||
| names(formals(CsvReadOptions$create)), | ||
| names(formals(readr_to_csv_read_options)) | ||
| ) | ||
|
|
||
| # We only currently support all of the readr options for parseoptions | ||
| parse_opts <- c( | ||
| supported_parse_opts <- c( | ||
| names(formals(CsvParseOptions$create)), | ||
| names(formals(readr_to_csv_parse_options)) | ||
| ) | ||
|
|
||
| opt_names <- names(opts) | ||
|
|
||
| # Catch any readr-style options specified with full option names that are | ||
| # supported by read_delim_arrow() (and its wrappers) but are not yet | ||
| # supported here | ||
| unsup_readr_opts <- setdiff( | ||
| names(formals(read_delim_arrow)), | ||
| c(convert_opts, read_opts, parse_opts, "schema") | ||
| c(supported_convert_opts, supported_read_opts, supported_parse_opts, "schema") | ||
| ) | ||
|
|
||
| is_unsup_opt <- opt_names %in% unsup_readr_opts | ||
|
|
@@ -228,9 +225,32 @@ check_csv_file_format_args <- function(...) { | |
| call. = FALSE | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| get_opt_names <- function(args) { | ||
| opt_names <- names(args) | ||
|
|
||
| # extract names of parse_options, read_options, and convert_options | ||
| if ("parse_options" %in% names(args) && is.list(args[["parse_options"]])) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens if
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I thought about documenting this, but I don't know where it would go - I don't think inside this function?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You could redocument
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If it's off-label usage then you probably should error for it...with the new function we have the opportunity to protect users from off-label usage (which was very very easy to do with the existing
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see...let's consider that later, then, but make it clear in this function that those are the two options for future us trying to read this code.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added more info here, and more explicit acknowledgement of this to |
||
| opt_names <- c(opt_names, names(args[["parse_options"]])) | ||
| } | ||
|
|
||
| if ("read_options" %in% names(args) && is.list(args[["read_options"]])) { | ||
| opt_names <- c(opt_names, names(args[["read_options"]])) | ||
| } | ||
|
|
||
| if ("convert_options" %in% names(args) && is.list(args[["convert_options"]])) { | ||
| opt_names <- c(opt_names, names(args[["convert_options"]])) | ||
| } | ||
|
|
||
| setdiff(opt_names, c("parse_options", "read_options", "convert_options")) | ||
| } | ||
|
|
||
| check_unrecognised_args <- function(opts) { | ||
| # Catch any options with full or partial names that do not match any of the | ||
| # recognized Arrow C++ option names or readr-style option names | ||
| opt_names <- get_opt_names(opts) | ||
|
|
||
| arrow_opts <- c( | ||
| names(formals(CsvParseOptions$create)), | ||
| names(formals(CsvReadOptions$create)), | ||
|
|
@@ -240,7 +260,8 @@ check_csv_file_format_args <- function(...) { | |
|
|
||
| readr_opts <- c( | ||
| names(formals(readr_to_csv_parse_options)), | ||
| names(formals(readr_to_csv_read_options)) | ||
| names(formals(readr_to_csv_read_options)), | ||
| "na" | ||
| ) | ||
|
|
||
| is_arrow_opt <- !is.na(pmatch(opt_names, arrow_opts)) | ||
|
|
@@ -271,26 +292,74 @@ check_ambiguous_options <- function(passed_opts, opts1, opts2) { | |
| } | ||
| } | ||
|
|
||
| check_schema <- function(schema, column_names) { | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This function is all existing code which I just extracted out into its own function |
||
| if (!is.null(schema) && !inherits(schema, "Schema")) { | ||
| abort(paste0( | ||
| "`schema` must be an object of class 'Schema' not '", | ||
| class(schema)[1], | ||
| "'." | ||
| )) | ||
| } | ||
|
|
||
| schema_names <- names(schema) | ||
|
|
||
| if (!is.null(schema) && !identical(schema_names, column_names)) { | ||
| missing_from_schema <- setdiff(column_names, schema_names) | ||
| missing_from_colnames <- setdiff(schema_names, column_names) | ||
| message_colnames <- NULL | ||
| message_schema <- NULL | ||
| message_order <- NULL | ||
|
|
||
| if (length(missing_from_colnames) > 0) { | ||
| message_colnames <- paste( | ||
| oxford_paste(missing_from_colnames, quote_symbol = "`"), | ||
| "not present in `column_names`" | ||
| ) | ||
| } | ||
|
|
||
| if (length(missing_from_schema) > 0) { | ||
| message_schema <- paste( | ||
| oxford_paste(missing_from_schema, quote_symbol = "`"), | ||
| "not present in `schema`" | ||
| ) | ||
| } | ||
|
|
||
| if (length(missing_from_schema) == 0 && length(missing_from_colnames) == 0) { | ||
| message_order <- "`column_names` and `schema` field names match but are not in the same order" | ||
| } | ||
|
|
||
| abort( | ||
| c( | ||
| "Values in `column_names` must match `schema` field names", | ||
| x = message_order, | ||
| x = message_schema, | ||
| x = message_colnames | ||
| ) | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| # Support both readr-style option names and Arrow C++ option names | ||
| csv_file_format_parse_options <- function(...) { | ||
| csv_file_format_parse_opts <- function(...) { | ||
| opts <- list(...) | ||
| # Filter out arguments meant for CsvConvertOptions/CsvReadOptions | ||
| convert_opts <- names(formals(CsvConvertOptions$create)) | ||
| convert_opts <- c(names(formals(CsvConvertOptions$create)), "na", "convert_options") | ||
| read_opts <- c( | ||
| names(formals(CsvReadOptions$create)), | ||
| names(formals(readr_to_csv_read_options)) | ||
| names(formals(readr_to_csv_read_options)), | ||
| "read_options" | ||
| ) | ||
| opts[convert_opts] <- NULL | ||
| opts[read_opts] <- NULL | ||
| opts[["schema"]] <- NULL | ||
| opt_names <- names(opts) | ||
| opts[["parse_options"]] <- NULL | ||
| opt_names <- get_opt_names(opts) | ||
|
|
||
| arrow_opts <- c(names(formals(CsvParseOptions$create))) | ||
| readr_opts <- c(names(formals(readr_to_csv_parse_options))) | ||
|
|
||
| is_arrow_opt <- !is.na(pmatch(opt_names, arrow_opts)) | ||
| is_readr_opt <- !is.na(pmatch(opt_names, readr_opts)) | ||
|
|
||
| # Catch options with ambiguous partial names (such as "del") that make it | ||
| # unclear whether the user is specifying Arrow C++ options ("delimiter") or | ||
| # readr-style options ("delim") | ||
|
|
@@ -313,28 +382,39 @@ csv_file_format_parse_options <- function(...) { | |
| csv_file_format_convert_opts <- function(...) { | ||
| opts <- list(...) | ||
| # Filter out arguments meant for CsvParseOptions/CsvReadOptions | ||
| arrow_opts <- names(formals(CsvParseOptions$create)) | ||
| arrow_opts <- c(names(formals(CsvParseOptions$create)), "parse_options") | ||
| readr_opts <- names(formals(readr_to_csv_parse_options)) | ||
| read_opts <- c( | ||
| names(formals(CsvReadOptions$create)), | ||
| names(formals(readr_to_csv_read_options)) | ||
| names(formals(readr_to_csv_read_options)), | ||
| "read_options" | ||
| ) | ||
|
|
||
| opts[arrow_opts] <- NULL | ||
| opts[readr_opts] <- NULL | ||
| opts[read_opts] <- NULL | ||
| opts[["schema"]] <- NULL | ||
| opts[["convert_options"]] <- NULL | ||
|
|
||
| # map "na" to "null_values" | ||
| if ("na" %in% names(opts)) { | ||
| opts[["null_values"]] <- opts[["na"]] | ||
| opts[["na"]] <- NULL | ||
| } | ||
|
|
||
| do.call(CsvConvertOptions$create, opts) | ||
| } | ||
|
|
||
| csv_file_format_read_opts <- function(schema = NULL, ...) { | ||
| opts <- list(...) | ||
| # Filter out arguments meant for CsvParseOptions/CsvConvertOptions | ||
| arrow_opts <- names(formals(CsvParseOptions$create)) | ||
| arrow_opts <- c(names(formals(CsvParseOptions$create)), "parse_options") | ||
| readr_opts <- names(formals(readr_to_csv_parse_options)) | ||
| convert_opts <- names(formals(CsvConvertOptions$create)) | ||
| convert_opts <- c(names(formals(CsvConvertOptions$create)), "na", "convert_options") | ||
| opts[arrow_opts] <- NULL | ||
| opts[readr_opts] <- NULL | ||
| opts[convert_opts] <- NULL | ||
| opts[["read_options"]] <- NULL | ||
|
|
||
| opt_names <- names(opts) | ||
| arrow_opts <- c(names(formals(CsvReadOptions$create))) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.