Fix custom CSV dialect delimiter being silently overridden - #680
Open
agu2347 wants to merge 1 commit into
Open
Conversation
CSVFormat.import_set() and export_stream_set() unconditionally did
kwargs.setdefault('delimiter', cls.DEFAULT_DELIMITER) before calling
csv.reader()/csv.writer(). Python's csv module lets explicit fmtparams
(like delimiter) override the corresponding attribute of a passed
dialect, so this silently reset any delimiter configured on a custom
dialect back to the comma default -- even though the caller never
asked for a delimiter override, only a dialect.
Only apply the default delimiter when no dialect was supplied.
Fixes jazzband#622
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #622.
Problem
As diagnosed by @claudep in the issue thread:
CSVFormat.import_set()andexport_stream_set()both unconditionally runbefore calling
csv.reader(in_stream, **kwargs)/csv.writer(stream, **kwargs). Python'scsvmodule documents that when both adialectand explicit fmtparams (likedelimiter) are given, the fmtparams take precedence over the dialect's own attributes. So if a caller passes a customdialect(with, say, a colon delimiter) but no explicitdelimiter, thissetdefaultstill injectsdelimiter=','into kwargs, which then silently overrides the dialect's delimiter back to the default comma:Fix
Only apply the default delimiter when no
dialectwas supplied, in bothimport_set()andexport_stream_set():This leaves existing behavior unchanged for the common case (no dialect, optional explicit delimiter) and for explicit
dialect+ explicitdelimiter(still overrides, per csv module semantics -- that's a deliberate, explicit request). It only changes the case where adialectis passed without an explicitdelimiter, which now correctly uses the dialect's own delimiter instead of silently falling back to comma.Testing
Added
CSVTests.test_csv_import_custom_dialect_delimiter_not_overridden, covering both directions (import and export) with a colon-delimited custom dialect. Confirmed it fails with the exact symptom from the issue (data parsed as a single column) when reverted, and passes with the fix.Ran the full existing suite (
pytest tests/): 188 passed (187 baseline + this new test), with the same 2 pre-existingruff/ruff::formatfailures present identically before and after this change (unrelated formatting/lint issues elsewhere intests/test_tablib.py, not touched by this PR).