fix(xlsx): a blank cell should not rewrite the rest of its column - #2516
Lukas (L4XB) wants to merge 5 commits into
Conversation
`DataFrame.to_html` writes the string `NaN` into an empty cell, and pandas upcasts any column that holds one. A spreadsheet with a single blank cell was therefore converted with `NaN` where the blank is, `12.0` where the sheet says `12`, `1.0` where it says `TRUE`, and `NaT` where a date is missing. Read the sheets with `dtype=object` so a blank no longer changes the type of its neighbours, render the blanks with `na_rep=""`, and pass a per-cell formatter so the remaining values are rendered as themselves - including a date cell, which openpyxl returns as a `datetime` whose `str()` would append a midnight time the spreadsheet does not have. `.xls` keeps pandas' own inference, since xlrd stores every number as a double and there is no integer to preserve; it shares the renderer, so its blanks stop reading as `NaN` too. The same two lines in `markitdown-ocr`'s XLSX converter now go through that renderer as well.
microsoft#2506 turned `_read_xlsx_sheets` into a context manager and made `XlsxConverterWithOCR` a subclass of `XlsxConverter`. The fix now sits on that structure: both `read_excel` calls in `_read_xlsx_sheets` pass `dtype=object`, and the sheet loop in `XlsxConverter.convert` renders through `sheet_to_html`. `_xlsx_converter_with_ocr.py` is taken from main as is. It no longer reads or renders a sheet itself, so the fix reaches it through the parent.
The OCR converter renders its tables through `XlsxConverter` now, so the blank-cell fix applies to it. Its snapshots pinned the old `NaN` cells, and the inheritance test pinned the exact `read_excel` arguments. Both now expect the fixed behaviour. A new test converts a sheet with an image and a blank row between an int and a bool column. It checks that the OCR output keeps `12` and `True` and leaves the blank cells empty.
`.xls` kept pandas' own type inference on the grounds that xlrd stores every number as a double. But pandas turns a whole-number double back into an int, so a blank cell still re-typed its column (`12.0`, `1.0`). It also turned a date column into `datetime64`, and the shared date formatter raised on its `NaT`: "NaTType does not support time". An `.xls` file with a blank date cell failed to convert at all. Read `.xls` with `dtype=object`, like `.xlsx`. `test.xls` renders the same as before.
With `index=False`, pandas renders a float in an object column through its own `float_format` and never reaches `formatters`, so `1e-07` came out as `0.0` once the sheets were read with `dtype=object`. On main, which reads with inference, the same cell rendered as `1.000000e-07`. The table now passes `float_format`, which formats with the 15 significant digits Excel shows: `1e-07`, `3.14159265358979`, and `0.1 + 0.2` as `0.3`. Both the .xlsx and the .xls path render through `sheet_to_html`, so both are covered.
|
Merged
Two corrections to my own earlier claims, both found while re-checking:
Not covered here: a cell whose text is Measured at
Both runs import the package from this worktree (checked by printing I also mutated the change in ten ways — each of the three |
What
An XLSX sheet is rendered with
DataFrame.to_html(index=False). Two pandas defaults meet there:to_htmlwrites the stringNaNinto a cell that is empty (NaTfor a date,Nonefor a missing value),read_excelupcasts a column that holds one — anintcolumn becomesfloat64, aboolcolumn becomesfloat64.So a single blank cell rewrites every other cell in its column. Given this sheet —
markitdown produced:
A blank cell is extremely common — the very first thing in a real spreadsheet — and every one of those differences is wrong in a way a reader cannot undo:
NaNreads as a value,12.0is not what the sheet says,1.0has lost that the column is a yes/no, and2026-01-05 00:00:00invents a time.How
pd.read_excel(..., dtype=object)in both calls in_read_xlsx_sheetsand inXlsConverter, so a blank no longer changes the type of its neighbours. pandas' Excel readers already hand back each cell's own Python type (int,float,bool,datetime), so nothing is being guessed here — it is pandas' re-inference that is dropped.sheet_to_html(sheet), withna_rep=""and a per-cell formatter. pandas appliesna_repto the blanks and the formatter to everything else, so the column is never re-typed on the way out. A fractional number is the exception: withindex=False, pandas renders a float in an object column with its own display precision and skips the formatter, so1e-07came out as0.0(1.000000e-07on main). The table passesfloat_formatfor that, which keeps the 15 significant digits Excel shows.datetimefor a date cell, andstr()on it appends a midnight time. A date renders as2026-01-05, a datetime keeps its time.After:
XlsConvertergets the same two changes. xlrd stores every number as a double, but pandas turns a whole-number double back into anint, so a blank upcast an.xlscolumn just like an.xlsxone. With pandas' own inference a blank date cell also makes the columndatetime64, and the date formatter cannot render itsNaT, so such a file would fail withNaTType does not support time.test.xlsrenders byte-identical withdtype=object.Since #2506,
markitdown-ocr'sXlsxConverterWithOCRsubclassesXlsxConverterand renders its tables through it, so it gets the fix without a change to its source. Its tests pinned the old output: thetest_xlsx_converter.pysnapshots expected| NaN | NaN |rows, andtest_inherited_repairs_and_native_table_fixes_reach_ocrpinned the exactread_excelarguments. Both now expect the fixed behaviour, and a new test converts a sheet with an image and a blank row through the OCR converter.Test
packages/markitdown/tests/test_xlsx_blank_cells.py— eight tests, pinning the exact markdown. The.xlsone reads a small fixture,tests/test_files/test_blank_cells.xls, with a blank row between an int, a bool and a date column.maintests/test_xlsx_blank_cells.pymarkitdown-ocrtest_xlsx_converter.py+test_xlsx_inheritance.pyThe two that pass on both sides are the guards that must not move: a sheet with no blank cell in it renders exactly as before, and a cell holding HTML still round-trips exactly as before.
The existing fixtures are unchanged —
test.xlsxandtest.xlsproduce byte-identical markdown on both sides (they contain no blank cells), and both suites are green when run the way CI runs them:black(the pinned 23.7.0 from.pre-commit-config.yaml) reports every tracked Python file unchanged.Not in this PR
A cell whose text is
N/A,NULL,NaN,Noneor#N/Ais still read as missing, because of pandas' defaultna_values. It rendered asNaNon main and renders blank here. Keeping the text needskeep_default_na=False, which is left for a separate change; #2499 is also in that area.