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", {

Reply via email to