This is an automated email from the ASF dual-hosted git repository.
thisisnic pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new 6d3331f28e GH-33708: [R] read_csv_arrow()'s timestamp_parsers
parameter is a bit light on documentation and doesn't appear to do anything
(#51166)
6d3331f28e is described below
commit 6d3331f28e16a6594034b0752f91f3db94515d1d
Author: Nic Crane <[email protected]>
AuthorDate: Wed Sep 16 16:31:28 2026 -0500
GH-33708: [R] read_csv_arrow()'s timestamp_parsers parameter is a bit light
on documentation and doesn't appear to do anything (#51166)
### Rationale for this change
`timestamp_parsers` works, but the docs didn't say that supplying parsers
replaces the default ISO-8601 parser, or that a non-matching parser silently
falls through to a string column during type inference. Passing
`convert_options` alongside `timestamp_parsers` also silently discarded the
latter.
### What changes are included in this PR?
Rewrites the `timestamp_parsers` documentation and adds an example showing
how to keep ISO-8601 as a fallback. Adds a warning when `timestamp_parsers` is
ignored because `convert_options` was supplied, in both the reader and dataset
paths. Unskips the dataset test for this option.
### Are these changes tested?
Yes, new tests for the inference path, the ISO-8601 fallback, and the
warning.
### Are there any user-facing changes?
Yes, a new warning when `timestamp_parsers` is passed together with
`convert_options`.
* GitHub Issue: #33708
Authored-by: Nic Crane <[email protected]>
Signed-off-by: Nic Crane <[email protected]>
---
r/R/csv.R | 55 +++++++++++++++++++++++++------------
r/R/dataset-format.R | 6 ++++
r/man/CsvReadOptions.Rd | 14 ++++++----
r/man/csv_convert_options.Rd | 20 ++++++++++----
r/man/open_delim_dataset.Rd | 20 ++++++++++----
r/man/read_delim_arrow.Rd | 30 ++++++++++++++++----
r/tests/testthat/test-csv.R | 42 ++++++++++++++++++++++++++++
r/tests/testthat/test-dataset-csv.R | 19 ++++++++-----
8 files changed, 158 insertions(+), 48 deletions(-)
diff --git a/r/R/csv.R b/r/R/csv.R
index a0ecd48677..6506322a9f 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 5119688104..9344430093 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 320685b05c..1ee6ab9f9c 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 c61da51fb7..1099cfcbe5 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 b5960152aa..9feabf51e9 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 f946785e4a..f72b0d333b 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 8fb11c2a5e..e7da8abd5c 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 8e6f5aa6f3..83fe4f6475 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", {