From e33447ed2844f4abc629d03c8b668bc4b83529a5 Mon Sep 17 00:00:00 2001 From: Kwak Byoung Min Date: Mon, 15 Jun 2026 13:19:59 +0900 Subject: [PATCH 1/4] Support csv quoting modes added in Python 3.12 Implement writer handling for QUOTE_STRINGS and QUOTE_NOTNULL so the exported constants no longer panic and follow CPython behavior for string, non-null, None, and quotechar validation cases. Constraint: RustPython contribution policy requires CPython-compatible behavior through Rust-side changes and regression coverage outside Lib/. Rejected: Marking CPython Lib tests or hiding the constants | would preserve a runtime panic for supported public constants. Confidence: high Scope-risk: narrow Directive: Keep QUOTE_STRINGS and QUOTE_NOTNULL writer behavior aligned with CPython's csv module when extending dialect validation. Tested: cargo check -p rustpython-stdlib; cargo run --release -- extra_tests/snippets/stdlib_csv.py; cargo run --release -- -m test test_csv; cargo clippy; pre-commit run --all-files via temporary venv/PATH shim; cargo test --workspace --exclude rustpython_wasm --exclude rustpython-venvlauncher -- --test-threads=1 Not-tested: Remote CI and PR creation were not run because remote git operations require explicit approval. Assisted-by: Codex:gpt-5.5 --- crates/stdlib/src/csv.rs | 111 ++++++++++++++++++++++++++++- extra_tests/snippets/stdlib_csv.py | 25 +++++++ 2 files changed, 133 insertions(+), 3 deletions(-) diff --git a/crates/stdlib/src/csv.rs b/crates/stdlib/src/csv.rs index f728002768d..252ef74629b 100644 --- a/crates/stdlib/src/csv.rs +++ b/crates/stdlib/src/csv.rs @@ -466,8 +466,7 @@ mod _csv { QuoteStyle::All => Self::Always, QuoteStyle::Nonnumeric => Self::NonNumeric, QuoteStyle::None => Self::Never, - QuoteStyle::Strings => todo!(), - QuoteStyle::Notnull => todo!(), + QuoteStyle::Strings | QuoteStyle::Notnull => Self::Necessary, } } } @@ -661,7 +660,7 @@ mod _csv { |_| { vm.new_type_error(r#""quotechar" must be a 1-character string"#) } )?)), PyNone => { - if let Some(QuoteStyle::All) = res.quoting { + if matches!(res.quoting, Some(quoting) if quoting != QuoteStyle::None) { return Err(ArgumentError::Exception( vm.new_type_error("quotechar must be set if quoting enabled"), )); @@ -1114,6 +1113,55 @@ mod _csv { } } + fn write_quoted_field( + output: &mut Vec, + data: &[u8], + dialect: PyDialect, + vm: &VirtualMachine, + ) -> PyResult<()> { + let quotechar = dialect + .quotechar + .ok_or_else(|| vm.new_type_error("quotechar must be set if quoting enabled"))?; + output.push(quotechar); + for &byte in data { + if byte == quotechar { + if dialect.doublequote { + output.push(quotechar); + output.push(quotechar); + } else if let Some(escapechar) = dialect.escapechar { + output.push(escapechar); + output.push(byte); + } else { + return Err(new_csv_error(vm, "need to escape, but no escapechar set")); + } + } else { + if dialect.escapechar == Some(byte) { + output.push(byte); + } + output.push(byte); + } + } + output.push(quotechar); + Ok(()) + } + + fn field_needs_quotes(data: &[u8], dialect: PyDialect) -> bool { + data.iter().any(|&byte| { + byte == dialect.delimiter + || dialect.quotechar == Some(byte) + || matches!(byte, b'\r' | b'\n') + || matches!(dialect.lineterminator, Terminator::Any(t) if byte == t) + }) + } + + fn write_lineterminator(output: &mut Vec, terminator: Terminator) { + match terminator { + Terminator::CRLF => output.extend_from_slice(b"\r\n"), + Terminator::Any(byte) => output.push(byte), + _ => unreachable!(), + } + } + #[pyclass(flags(DISALLOW_INSTANTIATION))] impl Writer { #[pygetset(name = "dialect")] @@ -1121,8 +1169,65 @@ mod _csv { self.dialect } + fn writerow_quoted_strings(&self, row: PyObjectRef, vm: &VirtualMachine) -> PyResult { + let _state = self.state.lock(); + let row: ArgIterable = ArgIterable::try_from_object(vm, row.clone()).map_err(|_e| { + new_csv_error( + vm, + format!("'{}' object is not iterable", row.class().name()), + ) + })?; + let fields: Vec<_> = row.iter(vm)?.collect::>()?; + let single_field = fields.len() == 1; + let mut output = Vec::new(); + + for (index, field) in fields.into_iter().enumerate() { + if index > 0 { + output.push(self.dialect.delimiter); + } + + let stringified; + let (data, is_str, is_none): (&[u8], bool, bool) = match_class!(match field { + ref s @ PyStr => (s.as_bytes(), true, false), + crate::builtins::PyNone => (b"", false, true), + ref obj => { + stringified = obj.str(vm)?; + (stringified.as_bytes(), false, false) + } + }); + + let should_quote = match self.dialect.quoting { + QuoteStyle::Strings => is_str || field_needs_quotes(data, self.dialect), + QuoteStyle::Notnull => !is_none, + _ => unreachable!(), + }; + if should_quote { + write_quoted_field(&mut output, data, self.dialect, vm)?; + } else if single_field && data.is_empty() { + return Err(new_csv_error( + vm, + "single empty field record must be quoted", + )); + } else { + output.extend_from_slice(data); + } + } + + write_lineterminator(&mut output, self.dialect.lineterminator); + let s = core::str::from_utf8(&output) + .map_err(|_| vm.new_unicode_decode_error("csv not utf8"))?; + self.write.call((s,), vm) + } + #[pymethod] fn writerow(&self, row: PyObjectRef, vm: &VirtualMachine) -> PyResult { + if matches!( + self.dialect.quoting, + QuoteStyle::Strings | QuoteStyle::Notnull + ) { + return self.writerow_quoted_strings(row, vm); + } + let mut state = self.state.lock(); let WriteState { buffer, writer } = &mut *state; diff --git a/extra_tests/snippets/stdlib_csv.py b/extra_tests/snippets/stdlib_csv.py index eb3461e9082..aa7b41223b6 100644 --- a/extra_tests/snippets/stdlib_csv.py +++ b/extra_tests/snippets/stdlib_csv.py @@ -1,4 +1,5 @@ import csv +import io from testutils import assert_raises @@ -47,3 +48,27 @@ def test_delim(): test_delim() + + +def test_quote_strings_and_notnull_writer(): + string_buf = io.StringIO() + csv.writer(string_buf, quoting=csv.QUOTE_STRINGS).writerow(["x", 1, None, ""]) + assert string_buf.getvalue() == '"x",1,,""\r\n' + + notnull_buf = io.StringIO() + csv.writer(notnull_buf, quoting=csv.QUOTE_NOTNULL).writerow(["x", 1, None, ""]) + assert notnull_buf.getvalue() == '"x","1",,""\r\n' + + for quoting in (csv.QUOTE_STRINGS, csv.QUOTE_NOTNULL): + buf = io.StringIO() + csv.writer(buf, quoting=quoting).writerow([None, None]) + assert buf.getvalue() == ",\r\n" + + with assert_raises(csv.Error): + csv.writer(io.StringIO(), quoting=quoting).writerow([None]) + + with assert_raises(TypeError): + csv.writer(io.StringIO(), quoting=quoting, quotechar=None) + + +test_quote_strings_and_notnull_writer() From 2c6b5ba44aca82aad75e2af6192509ff3c9703c0 Mon Sep 17 00:00:00 2001 From: Kwak Byoung Min <67214970+doma17@users.noreply.github.com> Date: Mon, 15 Jun 2026 23:15:54 +0900 Subject: [PATCH 2/4] Update crates/stdlib/src/csv.rs Co-authored-by: Shahar Naveh <50263213+ShaharNaveh@users.noreply.github.com> --- crates/stdlib/src/csv.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/stdlib/src/csv.rs b/crates/stdlib/src/csv.rs index 252ef74629b..e9e6ac2184d 100644 --- a/crates/stdlib/src/csv.rs +++ b/crates/stdlib/src/csv.rs @@ -660,7 +660,7 @@ mod _csv { |_| { vm.new_type_error(r#""quotechar" must be a 1-character string"#) } )?)), PyNone => { - if matches!(res.quoting, Some(quoting) if quoting != QuoteStyle::None) { + if res.quoting.is_some_and(|quoting| quoting != QuoteStyle::None) { return Err(ArgumentError::Exception( vm.new_type_error("quotechar must be set if quoting enabled"), )); From 7b58fffd63fbb2c52c0847d0a69364c37506cce9 Mon Sep 17 00:00:00 2001 From: Kwak Byoung Min <67214970+doma17@users.noreply.github.com> Date: Mon, 15 Jun 2026 23:16:07 +0900 Subject: [PATCH 3/4] Update crates/stdlib/src/csv.rs Co-authored-by: Shahar Naveh <50263213+ShaharNaveh@users.noreply.github.com> --- crates/stdlib/src/csv.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/stdlib/src/csv.rs b/crates/stdlib/src/csv.rs index e9e6ac2184d..909d17707ea 100644 --- a/crates/stdlib/src/csv.rs +++ b/crates/stdlib/src/csv.rs @@ -1177,7 +1177,7 @@ mod _csv { format!("'{}' object is not iterable", row.class().name()), ) })?; - let fields: Vec<_> = row.iter(vm)?.collect::>()?; + let fields = row.iter(vm)?.collect::>>()?; let single_field = fields.len() == 1; let mut output = Vec::new(); From 031bd14d4a77b7b148798859e3fef85df6155e50 Mon Sep 17 00:00:00 2001 From: Kwak Byoung Min Date: Tue, 16 Jun 2026 11:46:13 +0900 Subject: [PATCH 4/4] Keep csv writer follow-up lint clean Apply maintainer-requested expression cleanups and rustfmt wrapping so the PR lint job can pass without changing behavior. Constraint: GitHub Actions Lint failed on rustfmt output after the review suggestion. Rejected: Changing csv writer semantics | the failing check was formatting-only and all behavior tests already passed. Confidence: high Scope-risk: narrow Directive: Keep future review suggestions rustfmt-clean before pushing. Tested: cargo fmt --check; cargo check -p rustpython-stdlib; cargo run --release -- extra_tests/snippets/stdlib_csv.py; cargo run --release -- -m test test_csv; cargo clippy; pre-commit run --all-files; cargo test --workspace --exclude rustpython_wasm --exclude rustpython-venvlauncher -- --test-threads=1 Not-tested: Remote CI has not rerun yet; this commit will trigger it after push. Assisted-by: Codex:gpt-5.5 --- crates/stdlib/src/csv.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/stdlib/src/csv.rs b/crates/stdlib/src/csv.rs index 909d17707ea..bd80cf7eccf 100644 --- a/crates/stdlib/src/csv.rs +++ b/crates/stdlib/src/csv.rs @@ -660,7 +660,10 @@ mod _csv { |_| { vm.new_type_error(r#""quotechar" must be a 1-character string"#) } )?)), PyNone => { - if res.quoting.is_some_and(|quoting| quoting != QuoteStyle::None) { + if res + .quoting + .is_some_and(|quoting| quoting != QuoteStyle::None) + { return Err(ArgumentError::Exception( vm.new_type_error("quotechar must be set if quoting enabled"), ));