Skip to content

fix(XLSXToDocument): keep cells whose text is a default NA string - #12951

Closed
mukktinaadh wants to merge 1 commit into
deepset-ai:mainfrom
mukktinaadh:fix/xlsx-preserve-literal-na-cell-text
Closed

mukktinaadh wants to merge 1 commit into
deepset-ai:mainfrom
mukktinaadh:fix/xlsx-preserve-literal-na-cell-text

Conversation

@mukktinaadh

Copy link
Copy Markdown

Fixes #12945.

Problem

XLSXToDocument reads sheets through pandas.read_excel with pandas' default NA-string
parsing, so cells holding text like NA or N/A become missing values:

>>> XLSXToDocument().run([ByteStream(workbook)])["documents"][0].content
',A,B\n1,Code,Status\n2,,OK\n3,,pending\n'

Those are real values (a region code, an explicit status), and the original text cannot be
recovered from the resulting Document.

Change

Two keys in the read_excel defaults:

  • keep_default_na=False — stop parsing NA strings as missing.
  • na_values=[""] — keep only genuinely empty cells as missing, so the existing
    missing-value handling still applies to them.

They are placed before **self.read_excel_kwargs, so a caller passing
read_excel_kwargs={"keep_default_na": True} still gets the old behaviour.

After:

>>> XLSXToDocument().run([ByteStream(workbook)])["documents"][0].content
',A,B\n1,Code,Status\n2,NA,OK\n3,N/A,pending\n'

Why both keys

keep_default_na=False on its own regresses an existing test. In that configuration blank
cells become "", so table_format_kwargs={"missingval": "N/A"} no longer reaches them and
test_run_markdown_missing_value fails. Adding na_values=[""] keeps blank cells as NaN,
which is what missingval relies on:

{"keep_default_na": False}                        -> ['NA', 'N/A', '']
{"keep_default_na": False, "na_values": [""]}     -> ['NA', 'N/A', nan]

Tests

Four tests added to test/components/converters/test_xlsx_to_document.py, covering csv and
markdown output, that genuinely empty cells are still missing, and that read_excel_kwargs
can restore the previous parsing.

Against the parent commit, three of them fail:

FAILED ...::test_run_preserves_literal_na_text
FAILED ...::test_run_preserves_literal_na_text_in_markdown
FAILED ...::test_run_keeps_genuinely_empty_cells_missing

After the change the file is 29 passed, with the pre-existing
test_run_markdown_missing_value still green.

Also checked: ruff check and ruff format --check clean at ruff 0.16.0 (the pinned
ruff-pre-commit rev), and a release note added under releasenotes/notes/.

`XLSXToDocument` read sheets with `pandas.read_excel` and pandas' default
NA-string parsing, so text values such as "NA" or "N/A" were turned into
missing cells. Those can be real content, like a region code or an explicit
status, and once the document is produced the original text is gone.

Only genuinely empty cells count as missing now, via `na_values=[""]`, so the
existing missing-value handling (`missingval` for markdown, empty fields for
csv) still applies to blank cells. `read_excel_kwargs` is applied after these
defaults, so passing `keep_default_na=True` restores the previous behaviour.

Fixes deepset-ai#12945
@mukktinaadh
mukktinaadh requested a review from a team as a code owner September 25, 2026 11:17
@mukktinaadh
mukktinaadh requested review from anakin87 and removed request for a team September 25, 2026 11:17
@vercel

vercel Bot commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

@mukktinaadh is attempting to deploy a commit to the deepset Team on Vercel.

A member of the Team first needs to authorize it.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@HaystackBot

Copy link
Copy Markdown
Contributor

Hi @mukktinaadh, thanks a lot for your contribution! 🙏

We noticed that the Contributor License Agreement (CLA) check (license/cla) hasn't passed yet, so we've temporarily moved this PR to draft and paused the review assignment.

To get your PR reviewed, please sign the CLA via the link in the license/cla check below (or in the CLA bot comment). As soon as the check turns green, this PR will automatically be marked ready for review again and a reviewer will be re-assigned.

@HaystackBot
HaystackBot removed the request for review from anakin87 September 25, 2026 12:40
@HaystackBot HaystackBot added the cla-pending PR is in draft until the contributor signs the CLA label Sep 25, 2026
@HaystackBot
HaystackBot marked this pull request as draft September 25, 2026 12:40
@HaystackBot

Copy link
Copy Markdown
Contributor

Hi @mukktinaadh, just a friendly reminder: this PR is still in draft because the Contributor License Agreement (CLA) hasn't been signed yet. We'd love to review your contribution! Please sign the CLA via the link in the license/cla check, and this PR will automatically be marked ready for review.

@julian-risch

Copy link
Copy Markdown
Member

Thank you for your efforts! We're closing this PR because it makes the same XLSXToDocument NA-string fix as #13044, and the CLA has been unsigned since September 25.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-pending PR is in draft until the contributor signs the CLA topic:tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

XLSXToDocument drops literal NA and N/A cell values

4 participants