diff --git a/r/R/csv.R b/r/R/csv.R index a0ecd48677a8..6506322a9f6c 100644 --- a/r/R/csv.R +++ b/r/R/csv.R @@ -118,16 +118,24 @@ #' `TRUE`, blank rows will not be represented at all. If `FALSE`, they will be #' filled with missings. #' @param skip Number of lines to skip before reading data. -#' @param timestamp_parsers User-defined timestamp parsers. If more than one -#' parser is specified, the CSV conversion logic will try parsing values -#' starting from the beginning of this vector. Possible values are: +#' @param timestamp_parsers User-defined timestamp parsers, tried in order +#' when inferring column types and when converting columns of type +#' [timestamp()]. Possible values are: #' - `NULL`: the default, which uses the ISO-8601 parser #' - a character vector of [strptime][base::strptime()] parse strings -#' - a list of [TimestampParser] objects +#' - a list of [TimestampParser] objects and/or parse strings +#' +#' Supplying parsers replaces the default ISO-8601 parser rather than adding +#' to it. If none of the parsers match a value during type inference, the +#' column is read as a string without error; to get an error instead, specify +#' the column as a timestamp in `col_types`. These parsers are not used for +#' date columns. #' @param parse_options see [CSV parsing options][csv_parse_options()]. #' If given, this overrides any #' parsing options provided in other arguments (e.g. `delim`, `quote`, etc.). -#' @param convert_options see [CSV conversion options][csv_convert_options()] +#' @param convert_options see [CSV conversion options][csv_convert_options()]. +#' If given, this overrides any conversion options provided in other arguments +#' (e.g. `na`, `col_types`, `timestamp_parsers`, etc.). #' @param read_options see [CSV reading options][csv_read_options()] #' @param as_data_frame Should the function return a `tibble` (default) or #' an Arrow [Table]? @@ -161,6 +169,16 @@ #' col_types = schema(x = timestamp(unit = "us", timezone = "UTC")) #' ) #' +#' # Parse non-ISO timestamps with `timestamp_parsers`. Supplying parsers +#' # replaces the default ISO-8601 parser, so include `TimestampParser$create()` +#' # to keep it as a fallback: +#' write.csv( +#' data.frame(x = c("16/01/2023 19:47", "2023-01-17 08:00:00")), +#' file = tf, +#' row.names = FALSE +#' ) +#' read_csv_arrow(tf, timestamp_parsers = list("%d/%m/%Y %H:%M", TimestampParser$create())) +#' #' # Read directly from strings with `I()` #' read_csv_arrow(I("x,y\n1,2\n3,4")) #' read_delim_arrow(I(c("x y", "1 2", "3 4")), delim = " ") @@ -201,6 +219,12 @@ read_delim_arrow <- function( if (is.null(read_options)) { read_options <- readr_to_csv_read_options(skip, col_names) } + if (!is.null(convert_options) && !is.null(timestamp_parsers)) { + rlang::warn(c( + "`timestamp_parsers` is ignored when `convert_options` is supplied.", + i = "Pass it via `csv_convert_options(timestamp_parsers = ...)` instead." + )) + } if (is.null(convert_options)) { convert_options <- readr_to_csv_convert_options( na = na, @@ -535,17 +559,19 @@ csv_read_options <- function( #' columns named in it but not found in the data be included as a column of #' type `null()`? The default (`FALSE`) means that the reader will instead #' raise an error. -#' - `timestamp_parsers` User-defined timestamp parsers. If more than one -#' parser is specified, the CSV conversion logic will try parsing values -#' starting from the beginning of this vector. Possible values are -#' (a) `NULL`, the default, which uses the ISO-8601 parser; +#' - `timestamp_parsers` User-defined timestamp parsers, tried in order when +#' inferring column types and when converting timestamp columns. Possible +#' values are (a) `NULL`, the default, which uses the ISO-8601 parser; #' (b) a character vector of [strptime][base::strptime()] parse strings; or -#' (c) a list of [TimestampParser] objects. +#' (c) a list of [TimestampParser] objects and/or parse strings. Supplying +#' parsers replaces the default ISO-8601 parser; see [read_delim_arrow()] +#' for details. #' - `decimal_point` Character to use for decimal point in floating point numbers. Default: "." #' #' `TimestampParser$create()` takes an optional `format` string argument. #' See [`strptime()`][base::strptime()] for example syntax. -#' The default is to use an ISO-8601 format parser. +#' The default is to use an ISO-8601 format parser, which is useful as a +#' fallback at the end of a list of `timestamp_parsers`. #' #' The `CsvWriteOptions$create()` factory method takes the following arguments: #' - `include_header` Whether to write an initial header line with column names @@ -801,13 +827,8 @@ TimestampParser$create <- function(format = NULL) { #' columns named in it but not found in the data be included as a column of #' type `null()`? The default (`FALSE`) means that the reader will instead #' raise an error. -#' @param timestamp_parsers User-defined timestamp parsers. If more than one -#' parser is specified, the CSV conversion logic will try parsing values -#' starting from the beginning of this vector. Possible values are -#' (a) `NULL`, the default, which uses the ISO-8601 parser; -#' (b) a character vector of [strptime][base::strptime()] parse strings; or -#' (c) a list of [TimestampParser] objects. #' @param decimal_point Character to use for decimal point in floating point numbers. +#' @inheritParams read_delim_arrow #' #' @examplesIf arrow_with_dataset() #' tf <- tempfile() diff --git a/r/R/dataset-format.R b/r/R/dataset-format.R index 51196881040d..934443009302 100644 --- a/r/R/dataset-format.R +++ b/r/R/dataset-format.R @@ -232,6 +232,12 @@ check_csv_file_format_args <- function(args, partitioning = NULL) { args$read_options <- list(col_names = args$col_names) } + if (!is.null(args$convert_options) && !is.null(args$timestamp_parsers)) { + rlang::warn(c( + "`timestamp_parsers` is ignored when `convert_options` is supplied.", + i = "Pass it via `csv_convert_options(timestamp_parsers = ...)` instead." + )) + } if (is.null(args$convert_options)) { options$convert_options <- do.call(csv_file_format_convert_opts, c(args, list(read_options = options$read_options))) } else if (is.list(args$convert_options)) { diff --git a/r/man/CsvReadOptions.Rd b/r/man/CsvReadOptions.Rd index 320685b05c0d..1ee6ab9f9c35 100644 --- a/r/man/CsvReadOptions.Rd +++ b/r/man/CsvReadOptions.Rd @@ -88,18 +88,20 @@ CSV file that should be actually read and converted (in the vector's order). columns named in it but not found in the data be included as a column of type \code{null()}? The default (\code{FALSE}) means that the reader will instead raise an error. -\item \code{timestamp_parsers} User-defined timestamp parsers. If more than one -parser is specified, the CSV conversion logic will try parsing values -starting from the beginning of this vector. Possible values are -(a) \code{NULL}, the default, which uses the ISO-8601 parser; +\item \code{timestamp_parsers} User-defined timestamp parsers, tried in order when +inferring column types and when converting timestamp columns. Possible +values are (a) \code{NULL}, the default, which uses the ISO-8601 parser; (b) a character vector of \link[base:strptime]{strptime} parse strings; or -(c) a list of \link{TimestampParser} objects. +(c) a list of \link{TimestampParser} objects and/or parse strings. Supplying +parsers replaces the default ISO-8601 parser; see \code{\link[=read_delim_arrow]{read_delim_arrow()}} +for details. \item \code{decimal_point} Character to use for decimal point in floating point numbers. Default: "." } \code{TimestampParser$create()} takes an optional \code{format} string argument. See \code{\link[base:strptime]{strptime()}} for example syntax. -The default is to use an ISO-8601 format parser. +The default is to use an ISO-8601 format parser, which is useful as a +fallback at the end of a list of \code{timestamp_parsers}. The \code{CsvWriteOptions$create()} factory method takes the following arguments: \itemize{ diff --git a/r/man/csv_convert_options.Rd b/r/man/csv_convert_options.Rd index c61da51fb7ce..1099cfcbe5bb 100644 --- a/r/man/csv_convert_options.Rd +++ b/r/man/csv_convert_options.Rd @@ -51,12 +51,20 @@ columns named in it but not found in the data be included as a column of type \code{null()}? The default (\code{FALSE}) means that the reader will instead raise an error.} -\item{timestamp_parsers}{User-defined timestamp parsers. If more than one -parser is specified, the CSV conversion logic will try parsing values -starting from the beginning of this vector. Possible values are -(a) \code{NULL}, the default, which uses the ISO-8601 parser; -(b) a character vector of \link[base:strptime]{strptime} parse strings; or -(c) a list of \link{TimestampParser} objects.} +\item{timestamp_parsers}{User-defined timestamp parsers, tried in order +when inferring column types and when converting columns of type +\code{\link[=timestamp]{timestamp()}}. Possible values are: +\itemize{ +\item \code{NULL}: the default, which uses the ISO-8601 parser +\item a character vector of \link[base:strptime]{strptime} parse strings +\item a list of \link{TimestampParser} objects and/or parse strings +} + +Supplying parsers replaces the default ISO-8601 parser rather than adding +to it. If none of the parsers match a value during type inference, the +column is read as a string without error; to get an error instead, specify +the column as a timestamp in \code{col_types}. These parsers are not used for +date columns.} \item{decimal_point}{Character to use for decimal point in floating point numbers.} } diff --git a/r/man/open_delim_dataset.Rd b/r/man/open_delim_dataset.Rd index b5960152aac8..9feabf51e94e 100644 --- a/r/man/open_delim_dataset.Rd +++ b/r/man/open_delim_dataset.Rd @@ -172,18 +172,26 @@ filled with missings.} \item{skip}{Number of lines to skip before reading data.} -\item{convert_options}{see \link[=csv_convert_options]{CSV conversion options}} +\item{convert_options}{see \link[=csv_convert_options]{CSV conversion options}. +If given, this overrides any conversion options provided in other arguments +(e.g. \code{na}, \code{col_types}, \code{timestamp_parsers}, etc.).} \item{read_options}{see \link[=csv_read_options]{CSV reading options}} -\item{timestamp_parsers}{User-defined timestamp parsers. If more than one -parser is specified, the CSV conversion logic will try parsing values -starting from the beginning of this vector. Possible values are: +\item{timestamp_parsers}{User-defined timestamp parsers, tried in order +when inferring column types and when converting columns of type +\code{\link[=timestamp]{timestamp()}}. Possible values are: \itemize{ \item \code{NULL}: the default, which uses the ISO-8601 parser \item a character vector of \link[base:strptime]{strptime} parse strings -\item a list of \link{TimestampParser} objects -}} +\item a list of \link{TimestampParser} objects and/or parse strings +} + +Supplying parsers replaces the default ISO-8601 parser rather than adding +to it. If none of the parsers match a value during type inference, the +column is read as a string without error; to get an error instead, specify +the column as a timestamp in \code{col_types}. These parsers are not used for +date columns.} \item{quoted_na}{Should missing values inside quotes be treated as missing values (the default) or strings. (Note that this is different from the diff --git a/r/man/read_delim_arrow.Rd b/r/man/read_delim_arrow.Rd index f946785e4a41..f72b0d333b23 100644 --- a/r/man/read_delim_arrow.Rd +++ b/r/man/read_delim_arrow.Rd @@ -147,21 +147,29 @@ filled with missings.} If given, this overrides any parsing options provided in other arguments (e.g. \code{delim}, \code{quote}, etc.).} -\item{convert_options}{see \link[=csv_convert_options]{CSV conversion options}} +\item{convert_options}{see \link[=csv_convert_options]{CSV conversion options}. +If given, this overrides any conversion options provided in other arguments +(e.g. \code{na}, \code{col_types}, \code{timestamp_parsers}, etc.).} \item{read_options}{see \link[=csv_read_options]{CSV reading options}} \item{as_data_frame}{Should the function return a \code{tibble} (default) or an Arrow \link{Table}?} -\item{timestamp_parsers}{User-defined timestamp parsers. If more than one -parser is specified, the CSV conversion logic will try parsing values -starting from the beginning of this vector. Possible values are: +\item{timestamp_parsers}{User-defined timestamp parsers, tried in order +when inferring column types and when converting columns of type +\code{\link[=timestamp]{timestamp()}}. Possible values are: \itemize{ \item \code{NULL}: the default, which uses the ISO-8601 parser \item a character vector of \link[base:strptime]{strptime} parse strings -\item a list of \link{TimestampParser} objects -}} +\item a list of \link{TimestampParser} objects and/or parse strings +} + +Supplying parsers replaces the default ISO-8601 parser rather than adding +to it. If none of the parsers match a value during type inference, the +column is read as a string without error; to get an error instead, specify +the column as a timestamp in \code{col_types}. These parsers are not used for +date columns.} \item{decimal_point}{Character to use for decimal point in floating point numbers.} } @@ -260,6 +268,16 @@ read_csv_arrow( col_types = schema(x = timestamp(unit = "us", timezone = "UTC")) ) +# Parse non-ISO timestamps with `timestamp_parsers`. Supplying parsers +# replaces the default ISO-8601 parser, so include `TimestampParser$create()` +# to keep it as a fallback: +write.csv( + data.frame(x = c("16/01/2023 19:47", "2023-01-17 08:00:00")), + file = tf, + row.names = FALSE +) +read_csv_arrow(tf, timestamp_parsers = list("\%d/\%m/\%Y \%H:\%M", TimestampParser$create())) + # Read directly from strings with `I()` read_csv_arrow(I("x,y\n1,2\n3,4")) read_delim_arrow(I(c("x y", "1 2", "3 4")), delim = " ") diff --git a/r/tests/testthat/test-csv.R b/r/tests/testthat/test-csv.R index 8fb11c2a5e31..e7da8abd5ce9 100644 --- a/r/tests/testthat/test-csv.R +++ b/r/tests/testthat/test-csv.R @@ -760,3 +760,45 @@ test_that("altrep columns can roundtrip to table", { # we should still be able to turn this into a table expect_equal(tbl, as_tibble(arrow_table(new_df))) }) + +test_that("timestamp_parsers during type inference", { + tf <- tempfile() + on.exit(unlink(tf)) + writeLines(c("time", "16/01/2023 19:47"), tf) + expected <- as.POSIXct("2023-01-16 19:47:00", tz = "UTC") + + # A matching parser is used during type inference + df <- read_csv_arrow(tf, timestamp_parsers = "%d/%m/%Y %H:%M") + expect_equal(df$time, expected, ignore_attr = "tzone") + + # A non-matching parser falls through to string, without error + df <- read_csv_arrow(tf, timestamp_parsers = "%m-%d-%y") + expect_type(df$time, "character") + + # Supplying parsers replaces the ISO-8601 default... + writeLines(c("time", "16/01/2023 19:47", "2023-01-17 08:00:00"), tf) + df <- read_csv_arrow(tf, timestamp_parsers = "%d/%m/%Y %H:%M") + expect_type(df$time, "character") + + # ...unless TimestampParser$create() is included as a fallback + df <- read_csv_arrow( + tf, + timestamp_parsers = list("%d/%m/%Y %H:%M", TimestampParser$create()) + ) + expect_equal( + df$time, + as.POSIXct(c("2023-01-16 19:47:00", "2023-01-17 08:00:00"), tz = "UTC"), + ignore_attr = "tzone" + ) + + # timestamp_parsers is ignored, with a warning, when convert_options is supplied + expect_warning( + df <- read_csv_arrow( + tf, + convert_options = csv_convert_options(), + timestamp_parsers = "%d/%m/%Y %H:%M" + ), + "`timestamp_parsers` is ignored" + ) + expect_type(df$time, "character") +}) diff --git a/r/tests/testthat/test-dataset-csv.R b/r/tests/testthat/test-dataset-csv.R index 8e6f5aa6f310..83fe4f647540 100644 --- a/r/tests/testthat/test-dataset-csv.R +++ b/r/tests/testthat/test-dataset-csv.R @@ -628,17 +628,22 @@ test_that("open_delim_dataset params passed through to open_dataset", { expect_equal(ds$x, c(NA, 1L, NA, NA, 2L, NA, 3L)) # timestamp_parsers - skip("GH-33708: timestamp_parsers don't appear to be working properly") - dst_dir <- make_temp_dir() dst_file <- file.path(dst_dir, "data.csv") + writeLines(c("time", "16/01/2023 19:47"), dst_file) - df <- data.frame(time = "2023-01-16 19:47:57") - write.csv(df, dst_file, row.names = FALSE, quote = FALSE) - - ds <- open_csv_dataset(dst_dir, timestamp_parsers = c(TimestampParser$create(format = "%d-%m-%y"))) |> collect() + ds <- open_csv_dataset(dst_dir, timestamp_parsers = "%d/%m/%Y %H:%M") |> collect() + expect_equal(ds$time, as.POSIXct("2023-01-16 19:47:00", tz = "UTC"), ignore_attr = "tzone") - expect_equal(ds$time, "16-01-2023") + # timestamp_parsers is ignored, with a warning, when convert_options is supplied + expect_warning( + open_csv_dataset( + dst_dir, + convert_options = csv_convert_options(), + timestamp_parsers = "%d/%m/%Y %H:%M" + ), + "`timestamp_parsers` is ignored" + ) }) test_that("CSVReadOptions printing", {