fix(csv): detect the dialect when a quoted field spans several lines - #3985
fix(csv): detect the dialect when a quoted field spans several lines#3985r0h1tb wants to merge 2 commits into
Conversation
The dialect was sniffed from the first line alone. A quoted field containing a newline is cut mid-quote by that read, so the sniffer sees an unterminated quote, raises, and the backend falls back to a comma. Parsing the file with the wrong dialect under strict=True then aborts the conversion. Retry the sniff on a larger sample when the first line fails, which closes the quote. The first line is still tried first: the sniffer rejects samples whose rows hold different numbers of delimiters, and it infers quotechar from the sample too, so widening it unconditionally changes the dialect detected for ragged files. Signed-off-by: Rohit Behera <126186063+r0h1tb@users.noreply.github.com>
|
✅ DCO Check Passed Thanks @r0h1tb, all your commits are properly signed off. 🎉 |
Merge Protections🟢 Merge protection satisfied — ready to merge. Show 1 satisfied protection🟢 Enforce conventional commitMake sure that we follow https://www.conventionalcommits.org/en/v1.0.0/
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
| _log = logging.getLogger(__name__) | ||
|
|
||
| # Characters of the file handed to csv.Sniffer to detect the dialect. | ||
| _SNIFF_SAMPLE_SIZE: Final[int] = 65536 |
There was a problem hiding this comment.
The sniffer only needs to see one complete quoted field plus its surrounding delimiters. 64 KiB is 64× larger than the stdlib recommendation (1024 bytes) with no benefit. We could set _SNIFF_SAMPLE_SIZE to 4,096 bytes at most, staying in the same order of magnitude as the stdlib default while being clearly sufficient for any realistic multi-line quoted field.
| # Detect CSV dialect | ||
| head = self.content.readline() | ||
| self.content.seek(0) | ||
| sample = self.content.read(_SNIFF_SAMPLE_SIZE) |
There was a problem hiding this comment.
Minor performance comment: the sample is unconditionally read even when the first-line sniff will succeed. It could be avoided by reading the sample lazily, only after the head sniff fails.
Review feedback on docling-project#3985: 4 KiB is plenty for a multi-line quoted field and the sample is now only read when the first line fails to sniff. Signed-off-by: Rohit Behera <126186063+r0h1tb@users.noreply.github.com>
|
Both points addressed in a4f6ba9: sample capped at 4096, and it is now only read when the first-line sniff raises. |
2 participants
Problem
A CSV whose first field is quoted and contains a newline fails to convert:
convert()sniffs the dialect fromself.content.readline(). That read stops at the newline inside the quoted field, so the sniffer gets'"line one'— an unterminated quote — and raisescsv.Error. The handler falls back tocsv.excel, and reading a semicolon file as comma-delimited understrict=Trueaborts the conversion.Fix
Retry the sniff on a larger sample when the first line fails; the sample closes the quote and the dialect is detected.
The first line is still tried first, deliberately. Sniffing the larger sample unconditionally regresses existing fixtures:
csv.Snifferrejects samples whose rows carry different numbers of delimiters (csv-inconsistent-header,csv-too-many-columns), and it also infersquotecharfrom what it is given — oncsv-too-few-columnsit picks'and strips the quotes from a'b'cell, changing that file's groundtruth. Falling back only on failure leaves every currently-detected dialect untouched.Tests
test_quoted_newline_in_first_fieldintests/test_backend_csv.py. Against unfixed code:tests/test_backend_csv.pygoes 3 passed → 4 passed; no groundtruth was regenerated.ruff checkandruff format --checkare clean.Found while reading the backend, so there is no tracking issue. Related but distinct: #1716 is the single-column sniff failure already handled by the comma fallback.