Skip to content

Commit dfa76c3

Browse files
kingyzhangclaude
andcommitted
fix(excel_import): integers as integers, detach scatter rows, full test coverage
The 测试1.xlsx render still looked broken even after the previous fix: the 21 integer rows came out as `1.0, 1.0, 1.0, ...` and a stray `1, 2, , , , , ...` row sat awkwardly below the main block. Root causes: 1. **Float-promotion in render**. Pandas auto-promotes int columns to float64 when any cell is NaN, so `{{ row[col] }}` prints `1.0`. Fixed by emitting a smarter Jinja cell expression in the auto-generated present template: {% if v != v %}{# NaN → empty #} {% elif v is number and (v|int) == v %}{{ v|int }} {% else %}{{ v }}{% endif %} Plus a per-column int-detection pass on import: when every value in a column is a whole number (or empty), serialize as `1` not `1.0`. 2. **Detached scatter rows**. The 测试1.xlsx had `1, 2` in row 24 — below blank rows 22-23 — completely separated from the dense main block at rows 1-21. They were dragged into the data CSV, polluting dtypes (NaN → float promotion) and table layout. New _extract_sheet_data partitions body rows into "runs" of consecutive non-blank rows, picks the densest as the main block, and reroutes any other run whose density is < 2/3 of the main block's sparsest row into a `# Detached cells (...)` comment in compute. The data CSV stays clean. Test coverage on TestExcelImport went from 4 → 29 tests, organized by behaviour with explicit comments labelling each category. New cases: * smoke / contract: meta fields, Unicode filenames, valid grid * header detection: text / all-numeric / mixed / numeric strings * cell-type fidelity: strings, comma-quoted, ints (E2E render!), real floats, dates, bools, empty cells * formula extraction: plain, disabled flag, ArrayFormula, duplicate-pattern dedup * trailing summary rows: adjacent / with blank-row gap * detached scatter rows: filtered-with-density / clean stays clean * end-to-end render: no `1.0` strings in rendered <tbody> * multi-sheet / errors: selection, unknown sheet, multi sheets, FileNotFoundError, empty workbook Full suite: 471 passing (436 + 35 data_sources). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent a2fb4fb commit dfa76c3

2 files changed

Lines changed: 559 additions & 62 deletions

File tree

gridlang/excel_import.py

Lines changed: 152 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ def import_excel(
8888
all_formulas = {} # sheet_name → list of formula info
8989
all_styles = {} # sheet_name → style info
9090
all_summaries = {} # sheet_name → list of {coord, formula, value}
91+
all_extras = {} # sheet_name → list of {coord, value} (detached cells)
9192

9293
for sheet_name in target_sheets:
9394
ws_data = wb_data[sheet_name]
@@ -100,12 +101,14 @@ def import_excel(
100101
safe_name = _safe_sheet_name(sheet_name)
101102
parts.append(f"--- data:{safe_name} ---")
102103

103-
csv_content, summary_rows = _extract_sheet_data(ws_data, ws_formula)
104+
csv_content, summary_rows, extra_cells = _extract_sheet_data(ws_data, ws_formula)
104105
parts.append(csv_content)
105106
parts.append("")
106107

107108
if summary_rows:
108109
all_summaries[sheet_name] = summary_rows
110+
if extra_cells:
111+
all_extras[sheet_name] = extra_cells
109112

110113
# Collect formulas
111114
if ws_formula and include_formulas:
@@ -121,7 +124,9 @@ def import_excel(
121124

122125
# --- compute ---
123126
parts.append("--- compute ---")
124-
compute_code = _generate_compute_section(all_formulas, target_sheets, all_summaries)
127+
compute_code = _generate_compute_section(
128+
all_formulas, target_sheets, all_summaries, all_extras
129+
)
125130
parts.append(compute_code)
126131
parts.append("")
127132

@@ -217,11 +222,17 @@ def _is_summary_row(row: tuple, formula_row: Optional[tuple] = None) -> bool:
217222
return False
218223

219224

220-
def _extract_sheet_data(ws, ws_formula=None) -> tuple[str, list[dict]]:
221-
"""Extract sheet data as CSV string. Returns (csv, summary_rows).
225+
def _extract_sheet_data(ws, ws_formula=None) -> tuple[str, list[dict], list[dict]]:
226+
"""Extract sheet data as CSV string.
227+
228+
Returns ``(csv, summary_rows, extra_cells)``:
222229
223-
`summary_rows` is a list of {'coord', 'formula', 'value'} dicts for any
224-
formula-only trailing rows that were filtered out of the data CSV.
230+
* ``summary_rows`` — trailing single-cell formula rows (e.g.
231+
``=SUM(A1:I26)`` in A27) pulled out of the data block.
232+
* ``extra_cells`` — sparse rows that are *detached* from the main
233+
data block by a blank-row gap and have far lower fill density.
234+
They are kept for documentation in the compute section but
235+
removed from the data CSV so they don't pollute dtypes / layout.
225236
"""
226237
# Collect all rows up front so we can decide:
227238
# 1) whether row 1 is a header
@@ -267,36 +278,132 @@ def _extract_sheet_data(ws, ws_formula=None) -> tuple[str, list[dict]]:
267278

268279
summary_rows.reverse() # restore top-to-bottom order
269280

270-
# ── 2. Drop trailing blank rows that came BEFORE the now-last data row,
271-
# AND any rows we already trimmed.
272-
rows_in = raw_rows[:end]
273-
# Filter blank rows everywhere (preserves prior behaviour for the body).
274-
body_rows = [r for r in rows_in if not all(c is None for c in r)]
281+
# ── 2. Look at remaining rows. Keep their 1-based row numbers so we
282+
# can detect "detached" sparse rows separated by a blank gap.
283+
rows_in = [(r_idx + 1, raw_rows[r_idx]) for r_idx in range(end)]
284+
# Drop fully-blank rows up front; we record gaps via row numbers.
285+
body = [(rn, r) for rn, r in rows_in
286+
if not all(c is None or str(c).strip() == "" for c in r)]
287+
288+
if not body:
289+
return "", summary_rows, []
290+
291+
n_cols = max(len(r) for _, r in body)
292+
293+
def _density(row):
294+
non_empty = sum(1 for c in row if c is not None and str(c).strip() != "")
295+
return non_empty / n_cols if n_cols else 0.0
296+
297+
# ── 3. Find the main data block: the longest run of rows whose
298+
# row numbers are consecutive AND density is roughly uniform.
299+
# Anything detached from it by a blank-row gap with much lower
300+
# density is treated as a stray "extra" cell-set.
301+
runs = [] # list[list[int]] of indexes into `body`
302+
cur_run = [0]
303+
for i in range(1, len(body)):
304+
prev_rn, prev = body[i - 1]
305+
cur_rn, cur = body[i]
306+
if cur_rn == prev_rn + 1: # consecutive
307+
cur_run.append(i)
308+
else:
309+
runs.append(cur_run)
310+
cur_run = [i]
311+
runs.append(cur_run)
312+
313+
# Pick the "main" run = the one whose total fill (sum of densities)
314+
# is largest. That's robust against a single dense outlier row.
315+
main_run_idx = max(range(len(runs)),
316+
key=lambda k: sum(_density(body[i][1]) for i in runs[k]))
317+
main_run = set(runs[main_run_idx])
318+
# Lowest density in the main run is our floor — anything in a *separate*
319+
# run that's well below that floor is treated as a detached stray.
320+
main_run_densities = [_density(body[i][1]) for i in runs[main_run_idx]]
321+
main_floor = min(main_run_densities) if main_run_densities else 1.0
322+
323+
# ── 4. Walk body rows, separating main-block rows from detached extras.
324+
keep_rows = [] # list[(row_num, row_tuple)] for the data CSV
325+
extra_cells = [] # list[{'coord','value'}] for the compute annex
326+
for run_idx, run in enumerate(runs):
327+
if run_idx == main_run_idx:
328+
for i in run:
329+
keep_rows.append(body[i])
330+
continue
331+
# A run is "detached and noisy" when its mean density is well
332+
# below the sparsest main-block row — concretely, < 2/3 of the
333+
# main floor. Otherwise we keep it (e.g. user just had a blank
334+
# separator row above genuine data).
335+
run_density = sum(_density(body[i][1]) for i in run) / len(run)
336+
if run_density < (2.0 / 3.0) * main_floor:
337+
for i in run:
338+
rn, row = body[i]
339+
for col_idx, val in enumerate(row):
340+
if val is None or str(val).strip() == "":
341+
continue
342+
coord = f"{get_column_letter(col_idx + 1)}{rn}"
343+
extra_cells.append({"coord": coord, "value": val})
344+
else:
345+
for i in run:
346+
keep_rows.append(body[i])
275347

276-
if not body_rows:
277-
return "", summary_rows
348+
if not keep_rows:
349+
# Edge case: nothing made it past the filter — fall back to body.
350+
keep_rows = body
351+
extra_cells = []
278352

279-
# ── 3. Decide whether row 1 is a header or data.
280-
has_header = _looks_like_header_row(body_rows[0])
353+
# ── 5. Decide whether row 1 of keep_rows is a header or data.
354+
first_row = keep_rows[0][1]
355+
has_header = _looks_like_header_row(first_row)
281356
if has_header:
282-
header_row = body_rows[0]
283-
data_rows = body_rows[1:]
357+
header_row = first_row
358+
data_rows = [r for _, r in keep_rows[1:]]
284359
headers = [_clean_column_name(str(c) if c is not None else "") for c in header_row]
285360
else:
286361
# Synthesize col_A, col_B, ... headers from column count.
287-
n_cols = max(len(r) for r in body_rows)
288362
headers = [f"col_{get_column_letter(i + 1)}" for i in range(n_cols)]
289-
data_rows = body_rows
363+
data_rows = [r for _, r in keep_rows]
364+
365+
# ── 6. Detect per-column integer-ness so we don't have to write
366+
# `1.0`. We scan all kept data rows and emit ints when the
367+
# column's values are exclusively whole numbers (or empty).
368+
col_is_int = []
369+
for col in range(n_cols):
370+
all_int = True
371+
seen_any = False
372+
for row in data_rows:
373+
v = row[col] if col < len(row) else None
374+
if v is None or (isinstance(v, str) and v.strip() == ""):
375+
continue
376+
seen_any = True
377+
if isinstance(v, bool):
378+
# bools shouldn't drive int-ness either way
379+
all_int = False; break
380+
if isinstance(v, int):
381+
continue
382+
if isinstance(v, float):
383+
if v != v: # NaN
384+
continue
385+
if v == int(v):
386+
continue
387+
all_int = False; break
388+
# any non-numeric breaks the int run
389+
all_int = False; break
390+
col_is_int.append(seen_any and all_int)
290391

291-
# ── 4. Serialize.
392+
# ── 7. Serialize.
292393
csv_lines = [",".join(headers)]
293394
for row in data_rows:
294395
csv_row = []
295-
for cell in row:
396+
for col_idx, cell in enumerate(row):
296397
if cell is None:
297398
csv_row.append("")
298399
elif isinstance(cell, datetime):
299400
csv_row.append(cell.strftime("%Y-%m-%d"))
401+
elif col_idx < len(col_is_int) and col_is_int[col_idx] and isinstance(cell, (int, float)):
402+
# Whole number in an integer column → emit as int.
403+
if isinstance(cell, float) and cell != cell: # NaN
404+
csv_row.append("")
405+
else:
406+
csv_row.append(str(int(cell)))
300407
elif isinstance(cell, float) and cell == int(cell):
301408
csv_row.append(str(int(cell)))
302409
else:
@@ -306,7 +413,7 @@ def _extract_sheet_data(ws, ws_formula=None) -> tuple[str, list[dict]]:
306413
csv_row.append(val)
307414
csv_lines.append(",".join(csv_row))
308415

309-
return "\n".join(csv_lines), summary_rows
416+
return "\n".join(csv_lines), summary_rows, extra_cells
310417

311418

312419
def _clean_column_name(name: str) -> str:
@@ -616,15 +723,17 @@ def _generate_compute_section(
616723
all_formulas: dict,
617724
sheet_names: list[str],
618725
all_summaries: Optional[dict] = None,
726+
all_extras: Optional[dict] = None,
619727
) -> str:
620728
"""Generate compute section from extracted formulas."""
621729
lines = []
622730
lines.append("def transform(df):")
623731

624732
has_formulas = any(formulas for formulas in all_formulas.values())
625733
has_summaries = bool(all_summaries) and any(s for s in (all_summaries or {}).values())
734+
has_extras = bool(all_extras) and any(e for e in (all_extras or {}).values())
626735

627-
if has_formulas or has_summaries:
736+
if has_formulas or has_summaries or has_extras:
628737
lines.append(" # Auto-converted from Excel formulas")
629738
lines.append(" # Review and adjust as needed")
630739
lines.append("")
@@ -649,6 +758,19 @@ def _generate_compute_section(
649758
lines.append(f" # → {py_hint}")
650759
lines.append("")
651760

761+
# Detached "extra" cells — sparse rows that were below the main data block
762+
if has_extras:
763+
for sheet_name, extras in all_extras.items():
764+
if not extras:
765+
continue
766+
if len(all_extras) > 1:
767+
lines.append(f" # --- Detached cells for sheet: {sheet_name} ---")
768+
else:
769+
lines.append(" # --- Detached cells (separated from main data block by blank rows) ---")
770+
for e in extras:
771+
lines.append(f" # Cell {e['coord']}: {e['value']!r}")
772+
lines.append("")
773+
652774
if has_formulas:
653775
for sheet_name, formulas in all_formulas.items():
654776
if len(all_formulas) > 1:
@@ -789,7 +911,13 @@ def _generate_present_section(all_styles: dict, sheet_names: list[str]) -> str:
789911
lines.append(' {% for _, row in df.iterrows() %}')
790912
lines.append(' <tr>')
791913
lines.append(' {% for col in df.columns %}')
792-
lines.append(' <td>{{ row[col] }}</td>')
914+
# Pandas promotes "int columns with one NaN" to float64, so a clean
915+
# integer like 5 prints as "5.0". Strip the .0 in the template.
916+
# We also blank out NaN cells so they don't render as "nan".
917+
lines.append(' {% set v = row[col] %}')
918+
lines.append(' <td>{% if v is none or v != v %}{# NaN #}'
919+
'{% elif v is number and (v | int) == v %}{{ v | int }}'
920+
'{% else %}{{ v }}{% endif %}</td>')
793921
lines.append(' {% endfor %}')
794922
lines.append(' </tr>')
795923
lines.append(' {% endfor %}')

0 commit comments

Comments
 (0)