Skip to content

fix(docreader): keep spreadsheet cells that say N/A, None or NULL - #4452

Open
L4XB wants to merge 1 commit into
Tencent:mainfrom
L4XB:fix/excel-keep-literal-na-cells
Open

L4XB wants to merge 1 commit into
Tencent:mainfrom
L4XB:fix/excel-keep-literal-na-cells

Conversation

@L4XB

@L4XB L4XB commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

Description

ExcelParser reads each sheet with excel_file.parse(sheet_name=…, header=None), which uses pandas' default NA handling. That turns a cell whose text is N/A, NA, NULL, None, NaN, n/a or nan into a missing value. The row loop skips missing values, so the column: value pair disappears from the chunk, as if the cell were blank. These strings usually are the answer. None under "Allergies" means the patient has none, N/A means not applicable, NULL is what a database export writes, and NA is a region code. A header cell titled NA loses its label too, and falls back to the column letter.

Measured with the locked docreader environment (pandas 2.3.3, openpyxl 3.1.5) and xlsx_first_row_as_header=True, on a sheet with these cells:

row main this PR
P1: Allergies None, Region NA, Result N/A, Note ok Patient: P1,Note: ok Patient: P1,Allergies: None,Region: NA,Result: N/A,Note: ok
P2: Result NULL, Note blank Patient: P2,Allergies: Penicillin,Region: EU …,Region: EU,Result: NULL
P3: Allergies n/a, Region NaN, Note nan Patient: P3,Result: 42 Patient: P3,Allergies: n/a,Region: NaN,Result: 42,Note: nan

Changes in docreader/parser/excel_parser.py:

  • NA handling. _read_sheet_dataframe passes keep_default_na=False, so pandas keeps those strings as written.
  • Blank cells. A blank cell then arrives as "" instead of NaN. The row loop now skips "" as well as missing values, so a blank cell still adds no pair.
  • Empty rows. The df.dropna(how="all") pass is removed. With blanks as "" it no longer matches anything, and an all-blank row has no pairs, so the loop's existing if not page_content: continue already skips it. Row numbers in source_blocks are unchanged: they come from the index, which skipped rows keep.
  • Error cells. An Excel error cell such as #N/A still reads as missing, as before. pandas maps openpyxl error cells to NaN independently of na_values.

Scope: this is the docreader path. Per engines.go, xlsx goes to anydoc when the Go binary is built with it, so this parser runs with the builtin engine or when anydoc is not linked.

Type of Change

  • 🐛 Bug fix
  • ✨ New feature
  • 💥 Breaking change
  • 📚 Documentation update
  • 🎨 Refactor
  • ⚡ Performance improvement
  • 🧪 Test
  • 🔧 Configuration / Build / CI

Related Issue

None filed.

Testing

Three new cases in docreader/tests/test_excel_parser.py (ExcelParserTest):

  • test_cells_that_say_na_none_or_null_are_kept: the table above, in header mode. It fails on main.
  • test_a_header_cell_that_says_na_keeps_its_label: a header NA stays NA: 3. It fails on main.
  • test_blank_cells_and_blank_rows_are_still_skipped: a blank cell adds no pair, and an all-blank row adds no chunk. Its source_blocks row numbers stay [1, 2, 4]. This test passes on main too; it pins that the change does not alter blank handling. It fails if the "" check is dropped.
UV_PROJECT_ENVIRONMENT=… uv sync --project docreader --locked --no-dev --python 3.10.18
python -m unittest docreader.tests.test_excel_parser               # Ran 34 tests, OK (skipped=3)
python -m unittest discover -s docreader/tests -p "test_*.py"      # Ran 334 tests, 1 error: test_ssrf_proxy webkit, which needs the Playwright WebKit browser CI installs

It merges cleanly with the open #4160 (header labels) and #4447 (merge ranges), which touch nearby code.

Checklist

  • git diff --check origin/main...HEAD passes
  • Changed source files are formatted
  • Targeted tests for the changed packages/components pass
  • Diff-scoped lint passes where applicable (for Go: golangci-lint run --new-from-rev=origin/main ./...)
  • Full-repository checks were run, or any unrelated/environment-dependent failures are documented above
  • Self-reviewed the code
  • Added/updated tests covering the change
  • Updated related documentation (README, website-docs/, Swagger annotations, etc.)

ExcelParser read each sheet with pandas' default NA handling, which
turns a cell whose text is N/A, NA, NULL, None, NaN or n/a into a
missing value. The row loop skips missing values, so the pair vanished:
a patient with Allergies: None or a result marked N/A came out as if the
cell were blank, and a header cell titled NA lost its label.

The sheet is now read with keep_default_na=False. Blank cells arrive as
an empty string, so the row loop skips those as well, and the
dropna(how="all") pass is no longer needed: an empty row has no pairs
and was already skipped by the same check. Error cells such as #N/A
still read as missing.
Copilot AI balanced review requested due to automatic review settings October 17, 2026 22:03

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

This branch has not been deployed

No deployments
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.

2 participants