Skip to content

csv: apply dialect quoting and lineterminator in writer#8254

Open
jinmay wants to merge 2 commits into
RustPython:mainfrom
jinmay:csv-writer-dialect-quoting
Open

csv: apply dialect quoting and lineterminator in writer#8254
jinmay wants to merge 2 commits into
RustPython:mainfrom
jinmay:csv-writer-dialect-quoting

Conversation

@jinmay

@jinmay jinmay commented Jul 11, 2026 •
edited
Loading

Copy link
Copy Markdown

Summary

csv.writer ignored the quoting and lineterminator defined by a dialect (passed by name like dialect='unix' or as a dialect object), so writing with a built-in dialect diverged from CPython. Only explicit keyword arguments took effect. to_writer now resolves both from the dialect (via get_quoting() / get_lineterminator(), mirroring the existing get_delimiter()), while an explicit keyword argument still overrides it.

While fixing this I also found the QuoteStyle::Minimal → csv_core mapping was Always (quoting every field under QUOTE_MINIMAL); it is now Necessary. This was dormant because the quote style was only applied for an explicit quoting= argument, so it had to be fixed before applying the dialect's quoting.

import csv, io
sio = io.StringIO()
csv.writer(sio, dialect='unix').writerow([1, 'abc def', 'abc'])
# before: '1,abc def,abc\r\n'
# after:  '"1","abc def","abc"\n'   (matches CPython)

Test plan

  • Unmarks TestDialectUnix.test_simple_writer and Test_Csv.test_write_quoting (both @unittest.expectedFailure before). cargo run --release -- -m test test_csv: 128 run, 7 skipped, SUCCESS.
  • extra_tests/snippets/stdlib_csv.py: passes.
  • cargo fmt --check / cargo clippy: clean (no new warnings).
  • Compared against real CPython 3.14.6 on the same machine; the outputs for the dialect='unix' and QUOTE_MINIMAL cases match.

Assisted-by: Claude Code:claude-opus-4-8

jinmay added 2 commits July 11, 2026 15:54
@jinmay
csv: map QUOTE_MINIMAL to Necessary quote style
2afe107 
QuoteStyle::Minimal was mapped to csv_core's Always, which quoted every
field even under QUOTE_MINIMAL. Map it to Necessary so only fields that
require quoting are quoted.

Unmarks Test_Csv.test_write_quoting.

Assisted-by: Claude Code:claude-opus-4-8
@jinmay
csv: apply dialect quoting and lineterminator in writer
7145002 
csv.writer ignored the quoting and lineterminator from a dialect (whether
passed by name or object), always using the defaults. Resolve both from the
dialect via get_quoting()/get_lineterminator(), mirroring get_delimiter(), so
an explicit keyword argument still overrides the dialect.

Unmarks TestDialectUnix.test_simple_writer.

Assisted-by: Claude Code:claude-opus-4-8
@coderabbitai

coderabbitai Bot commented Jul 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

CSV writer configuration now respects dialect-provided quoting and line terminators, while explicit FormatOptions values override dialect defaults. Minimal quoting maps to csv_core::QuoteStyle::Necessary.

Changes

CSV formatting behavior

Layer / File(s) Summary
Quote mapping and option resolution
crates/stdlib/src/csv.rs		 Minimal quoting uses the necessary quote style, and helper methods resolve dialect or default values with explicit overrides.
Writer configuration
crates/stdlib/src/csv.rs		 to_writer applies resolved line terminator and quoting settings while retaining conditional escape handling.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested reviewers: youknowone, ShaharNaveh

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The code now applies dialect quoting and lineterminator in csv.writer and fixes QUOTE_MINIMAL mapping as required.
Out of Scope Changes check ✅ Passed The diff appears focused on the CSV writer behavior described in the issue with no unrelated changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main writer change: applying dialect quoting and line terminators.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

📦 Library Dependencies

The following Lib/ modules were modified. Here are their dependencies:

[x] lib: cpython/Lib/csv.py
[x] test: cpython/Lib/test/test_csv.py (TODO: 25)

dependencies:

  • csv

dependent tests: (4 tests)

  • csv: test_csv test_genericalias
    • importlib.metadata: test_importlib test_zoneinfo

Legend:

  • [+] path exists in CPython
  • [x] up-to-date, [ ] outdated

@jinmay jinmay marked this pull request as ready for review July 11, 2026 15:29
@youknowone youknowone added the z-ca-2026 Tag to track Contribution Academy 2026 label Jul 12, 2026
@doma17

doma17 commented Jul 12, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Hello!

With this PR, RustPython writes \r,\n!, while CPython writes "\r","\n"!.

  csv.writer(sio, lineterminator="!").writerow(["\r", "\n"])

It looks like embedded CR/LF may no longer be quoted when the line terminator is not CR or LF.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

z-ca-2026 Tag to track Contribution Academy 2026

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

csv.writer ignores a dialect's quoting and lineterminator

3 participants

@jinmay @doma17 @youknowone