DEV Community

Cover image for My import tool said row 347. The bug was on row 348
Adel Abdelaziz Goucem
Adel Abdelaziz Goucem

Posted on Edited on

My import tool said row 347. The bug was on row 348

#python #fastapi #pandas #opensource

My free HR dashboard makes one small, specific promise: when a row in your Excel file is invalid, the import report gives its exact line number, the one Excel shows. While recording a demo, I fed it a file whose row 348 was wrong. The report said row 347.

This post covers why that happened, the few lines that fixed it, and the tests that keep it from coming back. The code comes from HR Dashboard Lite, a free FastAPI and Pandas project (MIT).

A realistic export

HR exports are rarely clean tables. The file I used has a title line, a blank line, then the header, then the data:

Excel row Content
1 Fichier du personnel — données fictives (title)
2 (blank)
3 Matricule, Nom et prénom, Date d'embauche, Date de départ… (header)
4 first employee
348 hired 14/06/2021, left 01/03/2019: impossible, so the row must be rejected

The importer detects the header on its own, so the user never has to say where the data starts. That detection is where the line numbers went wrong.

The bug: blank rows vanished before anyone counted

The sheet is read with header=None, then cleaned up. Dropping fully empty rows looked harmless:

# before
raw = raw.dropna(how="all").reset_index(drop=True)
header_row = _find_header_row(raw)
frame = _apply_header(raw, header_row)
Enter fullscreen mode Exit fullscreen mode

Later, the report turned a position in frame into an Excel row number with arithmetic that assumed nothing had been removed:

excel_row = frame.index + header_row + 2
Enter fullscreen mode Exit fullscreen mode

reset_index(drop=True) had just renumbered everything from zero, so the blank row 2 was gone from the count. Every row after it was reported one line too early. With three blank rows, three lines too early. A file without blank rows gave correct numbers, which is why my own tests passed.

The fix: remember where each row came from

I still drop the empty rows, but I keep their original positions before resetting the index, and carry them all the way to the report:

# after
raw = raw.dropna(how="all")          # the index still holds each row's position in the sheet
positions = raw.index.to_numpy()
raw = raw.reset_index(drop=True)
header_row = _find_header_row(raw)
frame = _apply_header(raw, header_row)
excel_rows = [int(positions[i]) + 1 for i in frame.index]
Enter fullscreen mode Exit fullscreen mode

The table object now carries excel_rows, and the validation step reads from it instead of computing a number:

excel_row = pd.Index(excel_rows) if excel_rows is not None else frame.index + header_row + 2
Enter fullscreen mode Exit fullscreen mode

The old formula stays as a fallback for callers that pass a compact table. The rule I took from this: a row number is data, so it should be stored, not recomputed after the data has been reshaped.

The tests that pin it down

The test builds a workbook cell by cell, so the Excel row numbers are explicit, with blank rows before the header and between data:

def test_error_row_is_exact_with_blank_rows_before_and_between_data():
    content = _xlsx({
        1: ["Export du personnel"],
        # row 2 is blank
        3: HEADER,
        4: ["A1", "2020-01-01", None, "F"],
        # rows 5 and 6 are blank
        7: ["A2", "2021-05-01", "2019-01-01", "H"],   # exit before hire: Excel row 7
        8: ["A3", "2022-02-01", None, "F"],
        # row 9 is blank
        10: ["A4", "not a date", None, "F"],           # invalid hire date: Excel row 10
    })
    table = read_table(content, "export.xlsx")
    assert sorted(e["row"] for e in _errors(table)) == [7, 10]
Enter fullscreen mode Exit fullscreen mode

I added the same check for a compact sheet, and for a CSV with a title line and blank lines. The CSV case failed at first for a different reason, covered next.

Two more bugs, found by actually using it

Recording a phone-sized demo exposed two more problems that no unit test had caught.

A CSV with a title line crashed. pd.read_csv infers the number of columns from the first line, so a one-column title above a four-column header raised a ParserError. The fix is to give pandas the width of the widest line:

width = max((len(row) for row in csv.reader(io.StringIO(text), delimiter=delimiter)), default=1)
pd.read_csv(io.StringIO(text), sep=delimiter, header=None, dtype=object,
            skip_blank_lines=False, names=range(width))
Enter fullscreen mode Exit fullscreen mode

The column-mapping screen overflowed on phones. The page switched to a single column with grid-template-columns: 1fr, but 1fr means minmax(auto, 1fr). A wide preview table inside the card set the minimum width, so the whole card grew past the screen and the dropdowns were cut off. One line fixes it:

.import-grid { grid-template-columns: minmax(0, 1fr); }
Enter fullscreen mode Exit fullscreen mode

A small CSS test now checks that line, so a later refactor cannot quietly undo it.

What I would do again

  • Test with the ugly file first. My early tests used tidy tables, which is exactly the input that hides this bug.
  • Store identity, don't derive it. If users will see a row number, carry it with the row instead of rebuilding it after the data changed shape.
  • Use your own tool on a phone and on a messy export. Two of the three bugs here only showed up that way.

The free Lite edition (MIT) is on GitHub: Adel1637/hr-dashboard-lite. It includes the fixes above and 49 automated tests.

What is the worst Excel export you have ever had to import?

Top comments (3)

Collapse
 
csvtriage profile image
Lv Shao Mu •

"Store identity, don't derive it" is the right lesson, and carrying excel_rows through is the right fix. Two things I'd push on, because your CSV path has a different mapping problem than the width bug you already fixed.

1. In CSV, the physical line number is not the record number.

You report "the line number Excel shows". For xlsx that's well-defined. For CSV it isn't, because a quoted field can contain a newline — a notes column, an address, a pasted paragraph. Then one record spans several physical lines:

A1,Jean,"called back,
left message",2021-05-01
Enter fullscreen mode Exit fullscreen mode

That record is logical row 2 and physical lines 2–3. If a later record is invalid, the physical line number you report lands inside somebody else's field or one line past the real start, and skip_blank_lines=False doesn't help — those aren't blank lines. Whichever number you choose is defensible, but it has to be labelled, because the user is going to compare it against what the importer sees and those are different numbers.

The cheap check is to make the discrepancy visible rather than silent:

import csv, io
records = list(csv.reader(io.StringIO(text), delimiter=delimiter))
multiline = sum(1 for f in csv.reader(io.StringIO(text), delimiter=delimiter) if "\n" in "".join(f))
if multiline:
    warn("this file has fields containing line breaks; "
         "reported rows are record numbers, not physical lines")
Enter fullscreen mode Exit fullscreen mode

If it's zero, the two numbers coincide and the user never sees the caveat.

2. Your xlsx and CSV paths must be tested to agree with each other.

You now have two mappings: xlsx goes through excel_rows (or index + header_row + 2), CSV goes through read_csv(header=None, skip_blank_lines=False, names=range(width)). A user who re-saves their export as CSV must not suddenly get different row numbers for the same bad record — and this is exactly the kind of asymmetry that passes both test suites independently.

One assertion closes it:

def test_xlsx_and_csv_report_the_same_row(tmp_path):
    rows = {1: ["Export"], 3: HEADER, 4: GOOD, 7: BAD, 10: GOOD2}
    from_xlsx = [e["row"] for e in _errors(read_table(_xlsx(rows), "a.xlsx"))]
    from_csv  = [e["row"] for e in _errors(read_table(_xlsx_as_csv(rows), "a.csv"))]
    assert from_xlsx == from_csv
Enter fullscreen mode Exit fullscreen mode

3. Generate the blank rows instead of listing them.

Your fixtures pin rows 2, 5, 6, 9 — real cases, and they caught the bug. But the failure mode is "any blank line before the reported row shifts every number after it", which is a property, not a set of examples. A loop makes it exhaustive cheaply:

@pytest.mark.parametrize("blanks", [[], [2], [2, 5], [2, 5, 6], [2, 5, 6, 9], [3], [1, 2, 3]])
def test_blank_rows_never_shift_reported_numbers(blanks):
    content = _xlsx_with_blanks_at(blanks, bad_row=20)
    errors = _errors(read_table(content, "x.xlsx"))
    assert [e["row"] for e in errors] == [20]      # 20 is 20 regardless of what was removed
Enter fullscreen mode Exit fullscreen mode

That single parametrisation covers the whole class — including a blank line before the header, which is the case your original header_row + 2 arithmetic also got wrong and the fixture above doesn't reach.

The CSS minmax(0, 1fr) catch is a good reminder that "test it on a phone" finds things no unit test will.

Collapse
 
agstudio profile image
Adel Abdelaziz Goucem •

Thanks, great points. You're right about line vs record: the report gives the row as Excel shows it, so a multi-line quoted field counts as one row, which a text editor won't match. I'll label it "Excel row" and warn when a file has line breaks inside fields. I also checked CSV vs XLSX on the same bad record (same row number) and a grid of blank-line positions (all match), and both are going into a parametrized test.

Collapse
 
agstudio profile image
Adel Abdelaziz Goucem •

Update: all three are now in the repo, with an "Excel row" label, a warning for multi-line CSV cells and the parametrized CSV/XLSX tests: github.com/Adel1637/hr-dashboard-l...

⌂ 换网址