GH-38217: [R] Map format = "text" to format = "csv" in write_dataset() by thisisnic · Pull Request #50367 · apache/arrow

thisisnic · 2026-07-04T22:15:00Z

Rationale for this change

write_dataset() errors on format="text" even though open_dataset() allows it.

What changes are included in this PR?

Map "text" to "csv" so write_dataset() mirrors open_dataset()

Are these changes tested?

Yes

Are there any user-facing changes?

No

GitHub Issue: [R] Map format = "text to format = "csv" in write_dataset() #38217

github-actions · 2026-07-04T22:15:23Z

⚠️ GitHub issue #38217 has been automatically assigned in GitHub to PR creator.

Copilot

Pull request overview

This PR aligns the R write_dataset() format handling with open_dataset() by treating format = "text" as an alias of CSV, so users can write delimited text datasets with the same expectations as when opening them.

Changes:

Map format = "text" to "csv" inside write_dataset() so it uses CSV defaults (notably comma delimiter) and generates .csv filenames.
Narrow the “delimiter required” validation to only apply to format = "txt", avoiding the previous error for format = "text".
Update tests to cover the new format = "text" behavior and remove the old expectation that it errors.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
r/R/dataset-write.R	Adds `text -> csv` mapping and updates delimiter validation to only enforce a delimiter for `txt`.
r/tests/testthat/test-dataset-write.R	Replaces the previous error expectation for `format="text"` with a round-trip test asserting `.csv` output and correct data.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

r/R/dataset-write.R:156

PR description says there are no user-facing changes, but this change makes write_dataset(format = "text") succeed where it previously errored (a user-visible behavior change). Consider updating the PR description/release-note wording to reflect this bugfix.

  if (format == "text") {
    format <- "csv"
  }

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Copilot

🟢 Approval recommended

The change is narrowly scoped, matches the linked issue’s acceptance criteria, and is covered by an explicit regression test.

Review details

Files reviewed: 2/2 changed files
Comments generated: 0 new
Review effort level: Lite

jonkeane

I'm fine with this in principle but the distinction between "text" and "txt" (if I'm reading that correctly?) seems very strange.

jonkeane · 2026-09-07T22:10:39Z

-  if (format %in% c("txt", "text") && !any(c("delimiter", "delim") %in% names(dots))) {
+  if (format == "txt" && !any(c("delimiter", "delim") %in% names(dots))) {


Does this mean that "text" and "txt" will behave differently now? As in: "text" will automagically be CSV, but "txt" one needs to supply a delimiter (which might be a comma)?

Yeah, mapping "text" to "csv" is to mirror open_dataset() but it's a bit weird.

"txt" is what write_delim_dataset() passes internally and it always sends a delimiter, so I've left that check alone rather than also defaulting it to comma.

How bad would it be to make open_dataset() be consistent with this? (and also internally 😂 )?

Yeah, this is a super weird one, I think we can actually just totally get rid of "txt" given it was originally added so that write_delim_dataset() could force users to specify a delimiter, and isn't even documented, it just happens to work. We could probably clean up the API a bit by deprecating it tbh. I'll update the PR so both sides are symmetrical but without this weirdness!

Copilot

Copilot review overview

🟢 Approval recommended

No unresolved review issues were identified.

Review effort: Lite
Findings: None

Resolved since last review (2)

format = "text" is mapped to "csv" after basename_template's default has already been…
The test that asserted format = "text" errors was removed, but there’s no new assertion covering…

thisisnic requested a review from jonkeane as a code owner July 4, 2026 22:15

Copilot AI lite review requested due to automatic review settings July 4, 2026 22:15

github-actions Bot added the Component: R label Jul 4, 2026

Copilot started reviewing on behalf of thisisnic July 4, 2026 22:15 View session

github-actions Bot added the awaiting committer review Awaiting committer review label Jul 4, 2026

Copilot AI reviewed Jul 4, 2026

View reviewed changes

thisisnic mentioned this pull request Jul 5, 2026

[CI] Copilot auto-reviews trigger workflow approval requirement on pr_review_trigger.yml #50370

Closed

thisisnic marked this pull request as draft July 5, 2026 13:48

thisisnic changed the title ~~GH-38217: [R] Map format = "text to format = "csv" in write_dataset()~~ GH-38217: [R] Map format = "text" to format = "csv" in write_dataset() Aug 18, 2026

thisisnic force-pushed the worktree-GH-38217-text-to-csv branch from d048f45 to 4292634 Compare August 18, 2026 13:49

thisisnic marked this pull request as ready for review August 18, 2026 13:49

Copilot AI review requested due to automatic review settings August 18, 2026 13:49

Copilot started reviewing on behalf of thisisnic August 18, 2026 13:50 View session

Copilot AI reviewed Aug 18, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings August 25, 2026 10:07

thisisnic force-pushed the worktree-GH-38217-text-to-csv branch from 4292634 to 85d49ac Compare August 25, 2026 10:07

Copilot started reviewing on behalf of thisisnic August 25, 2026 10:08 View session

Copilot AI reviewed Aug 25, 2026

View reviewed changes

thisisnic commented Aug 25, 2026

View reviewed changes

Comment thread r/tests/testthat/test-dataset-write.R

Copilot AI review requested due to automatic review settings August 25, 2026 10:42

github-actions Bot added awaiting review Awaiting review awaiting changes Awaiting changes and removed awaiting committer review Awaiting committer review labels Aug 25, 2026

Copilot started reviewing on behalf of thisisnic August 25, 2026 10:42 View session

Copilot AI reviewed Aug 25, 2026

View reviewed changes

Comment thread r/tests/testthat/test-dataset-write.R Outdated

Comment thread r/R/dataset-write.R Outdated

Copilot AI review requested due to automatic review settings September 4, 2026 10:26

Copilot started reviewing on behalf of thisisnic September 4, 2026 10:27 View session

github-actions Bot removed awaiting review Awaiting review awaiting changes Awaiting changes labels Sep 4, 2026

github-actions Bot added the awaiting review Awaiting review label Sep 4, 2026

Copilot AI reviewed Sep 4, 2026

View reviewed changes

github-actions Bot added awaiting changes Awaiting changes and removed awaiting review Awaiting review labels Sep 4, 2026

jonkeane reviewed Sep 7, 2026

View reviewed changes

thisisnic added 5 commits October 4, 2026 22:55

Map text to csv

20bdb88

Add test

08218e3

Delete extraneous test

ccc0492

Add test

10338ee

Deprecated "txt"

71a2932

Copilot AI lite review requested due to automatic review settings October 4, 2026 22:22

thisisnic force-pushed the worktree-GH-38217-text-to-csv branch from f8b53f5 to 71a2932 Compare October 4, 2026 22:22

github-actions Bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Oct 4, 2026

Copilot started reviewing on behalf of thisisnic October 4, 2026 22:22 View session

github-actions Bot added awaiting changes Awaiting changes and removed awaiting change review Awaiting change review labels Oct 4, 2026

Copilot AI reviewed Oct 4, 2026

View reviewed changes

		if (format %in% c("txt", "text") && !any(c("delimiter", "delim") %in% names(dots))) {
		if (format == "txt" && !any(c("delimiter", "delim") %in% names(dots))) {

Conversation

thisisnic commented Jul 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

github-actions Bot commented Jul 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

jonkeane left a comment

Choose a reason for hiding this comment

Uh oh!

jonkeane Sep 7, 2026

Choose a reason for hiding this comment

Uh oh!

thisisnic Sep 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonkeane Sep 18, 2026

Choose a reason for hiding this comment

Uh oh!

thisisnic Oct 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟢 Approval recommended

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

thisisnic commented Jul 4, 2026 •

edited

Loading

thisisnic Sep 18, 2026 •

edited

Loading