Skip to content

fix(csv): read the separator the file was written with - #2517

Open
Lukas (L4XB) wants to merge 1 commit into
microsoft:mainfrom
L4XB:fix/csv-delimiter
Open

Lukas (L4XB) wants to merge 1 commit into
microsoft:mainfrom
L4XB:fix/csv-delimiter

Conversation

@L4XB

Copy link
Copy Markdown

What

CsvConverter parses every file with csv.reader(...), which means a comma. A .csv file is not always comma separated:

  • Excel writes the list separator of the machine's locale, and that is a semicolon across most of Europe — Save as → CSV on a German, French, Spanish or Italian Windows produces Name;Region;Units.
  • A tab-separated export is routinely saved with a .csv extension.

Parsing those with a comma does not fail. It returns one column holding the whole row, separators and all:

| Name;Region;Units |
| --- |
| Widget;EU;12 |
| Gadget;US;7 |

Every column boundary in the file is gone, and the markdown table claims the file has one column. Same for the tab case, where the raw tab ends up inside the cell.

How

  • The separator is picked among ,, ; and \t by the one that yields the same column count on every row of a sample (the first 20 non-blank rows, capped at 64 KB). A separator the file was not written with either does not occur at all, or occurs by accident and then the rows do not line up — which is what keeps a comma inside a sentence, or a semicolon inside a quoted field, from being read as one.
  • When no candidate is consistent, the comma is kept. An ambiguous file, a single-column file and a ragged file are therefore parsed exactly as they are today.
  • On a tie the comma wins, because it is tried first and a later candidate has to be strictly better.
  • A leading sep= line — which Excel writes and honours, and hides from the user — now sets the separator directly, and stops being emitted as a table row. It is read after the BOM is stripped, so a BOM-prefixed export works too.
| Name | Region | Units |
| --- | --- | --- |
| Widget | EU | 12 |
| Gadget | US | 7 |

Test

packages/markitdown/tests/test_csv_delimiter.py — nine tests. The same table is written three times, once per separator, and must convert to the same markdown.

main this branch
tests/test_csv_delimiter.py 4 failed, 5 passed 9 passed
# on main
FAILED test_the_separator_the_file_was_written_with_is_used[;]
FAILED test_the_separator_the_file_was_written_with_is_used[\t]
FAILED test_a_sep_directive_sets_the_separator_and_is_not_a_row
FAILED test_a_sep_directive_survives_a_bom

The five that pass on both sides are the guards that must not move: the comma case, a semicolon inside a quoted field, a single-column file whose values contain semicolons, a ragged comma file (the padding behaviour test_csv_blank_runs.py pins), and an empty file.

Full suite: 901 passed, 14 skipped, including test_csv_blank_runs.py and test_csv_line_endings.py unchanged.

black (the pinned 23.7.0 from .pre-commit-config.yaml) reports both files unchanged.

A .csv file is not always comma separated. Excel writes the list separator of
the machine's locale, which is a semicolon across most of Europe, and a
tab-separated export is routinely saved as .csv. Parsing either one with a comma
does not fail: it returns one column holding the whole row, separators included,
so every column boundary in the file is lost.

Detect the separator among comma, semicolon and tab by taking the one that
gives the same column count on every row of a sample, keeping the comma when
none of them does, so an ambiguous or ragged file is parsed exactly as before.
A leading `sep=` line, which Excel writes and honours, sets the separator
directly and is no longer emitted as a table row.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant