alistaire47 commented on code in PR #33614:
URL: https://github.com/apache/arrow/pull/33614#discussion_r1068366811
##########
r/R/dataset-format.R:
##########
@@ -118,102 +118,71 @@ IpcFileFormat <- R6Class("IpcFileFormat", inherit =
FileFormat)
#' @rdname FileFormat
#' @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],
- "'."
- ))
- }
+CsvFileFormat$create <- function(...) {
Review Comment:
whereby:
- AFAIK this isn't intended to be used much by users directly,
- but it is exported, so at least some power users probably will anyway,
- and I appreciate the flattening here,
- but whenever more parameters get shoved through `...` it gets harder for
users to figure out what they can specify there (and specifically here you'll
have to dig through more levels of source to figure it out),
suggested:
- could we document where to look in `?FileFormat` a bit more?
- could that page specify that`CsvFileFormat` exists if this is going to
`@rdname` there?
--
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]
