Copilot commented on code in PR #50367:
URL: https://github.com/apache/arrow/pull/50367#discussion_r3852185604
##########
r/tests/testthat/test-dataset-write.R:
##########
@@ -919,11 +919,6 @@ test_that("Writing a flat file dataset without a delimiter
throws an error.", {
write_dataset(df, dst_dir, format = "txt"),
"A delimiter must be given for a txt format."
)
-
- expect_error(
- write_dataset(df, dst_dir, format = "text"),
- "A delimiter must be given for a txt format."
- )
})
Review Comment:
The test that asserted `format = "text"` errors was removed, but there’s no
new assertion covering the intended behavior (that `format = "text"` succeeds
and behaves like CSV). Add a regression check so future changes don’t
reintroduce the delimiter error or break the alias.
##########
r/R/dataset-write.R:
##########
@@ -151,6 +151,9 @@ write_dataset <- function(
if (format %in% c("feather", "ipc")) {
format <- "arrow"
}
+ if (format == "text") {
+ format <- "csv"
+ }
Review Comment:
`format = "text"` is mapped to `"csv"` after `basename_template`'s default
has already been evaluated (it uses the original `format`). This means calling
`write_dataset(..., format = "text")` will write CSV data but (by default) name
files `part-*.text`, which is surprising and makes the alias incomplete.
--
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]
