fix(excel): preserve zero padding in clean_excel, as clean_csv already does

CHANGELOG.md

@@ -7,6 +7,20 @@ adheres to [Semantic Versioning](https://semver.org/).
 ## [Unreleased]
 
 ### Fixed
+- `fd.clean_excel` now preserves zero padding, as `fd.clean_csv` already did.
+  `pandas.read_excel` infers types exactly as `read_csv` does, so a cell that
+  the workbook stored as the **text** `"02134"` arrived as the integer `2134`
+  and the padding was gone before any cleaning step ran. `clean_csv` avoids
+  this with a bounded pre-scan; `clean_excel` had no equivalent, so
+  `preserve_leading_zeros=True` — documented as a shared option — changed
+  nothing there, and a postcode or account column was silently read as a
+  quantity and then profiled and outlier-checked as one. A `read_excel`
+  counterpart of the pre-scan now reads only the zero-padded numeric columns as
+  text. `preserve_leading_zeros=False` still opts out, an explicit
+  `read_excel_kwargs={"dtype": ...}` still wins, `sheet_name` is honoured, and
+  columns without padding keep their numeric dtype. **Default-output change:** a
+  zero-padded numeric column in a spreadsheet now cleans as text rather than
+  losing its padding.
 - An unrecognised `semantic_type` no longer receives *more* text cleaning than
   a recognised one. `textclean.config_for_field` matched the declared type
   exactly — case-sensitively and untrimmed — and fell through to the caller's

src/freshdata/_csv_io.py

@@ -1,4 +1,5 @@
-"""CSV read helpers shared by the CLI and :func:`freshdata.clean_csv`. Internal.
+"""Spreadsheet read helpers shared by the CLI, :func:`freshdata.clean_csv` and
+:func:`freshdata.clean_excel`. Internal.
 
 ``pandas.read_csv`` infers ``"02134"`` as the integer ``2134`` before any cleaning
 step runs, so ``CleanConfig.preserve_leading_zeros`` never gets a chance to keep
@@ -67,3 +68,50 @@ def leading_zero_dtypes(
         if safe_to_numeric(values, errors="coerce").notna().all():
             padded[column] = str
     return padded
+
+
+def leading_zero_dtypes_excel(
+    path: object,
+    *,
+    read_excel_kwargs: Mapping[str, Any] | None = None,
+    nrows: int = LEADING_ZERO_SCAN_ROWS,
+) -> dict[Hashable, type[str]]:
+    """``{column: str}`` for numeric-looking spreadsheet columns with zero padding.
+
+    ``pandas.read_excel`` infers types exactly as ``read_csv`` does, so a cell
+    that openpyxl stored as the *text* ``"02134"`` still arrives as the integer
+    ``2134`` and the padding is gone before any cleaning step runs. This is the
+    ``read_excel`` counterpart of :func:`leading_zero_dtypes`, and it applies the
+    same rule: all non-missing sampled values must parse as numbers, and at least
+    one must be zero-padded.
+
+    Returns ``{}`` when the caller already decides types via ``dtype`` or
+    ``converters``, when *path* is not a filesystem path, when the workbook
+    selects several sheets (there is no single column set to map), or when the
+    sample cannot be read — the real read then reports that error itself.
+    """
+    kwargs = dict(read_excel_kwargs or {})
+    if any(kwargs.get(key) is not None for key in _TYPE_OPTIONS):
+        return {}
+    if not isinstance(path, (str, os.PathLike)):
+        return {}
+    sheet = kwargs.get("sheet_name", 0)
+    if sheet is None or isinstance(sheet, (list, tuple)):
+        return {}  # several sheets: clean_excel rejects this case anyway
+    limit = kwargs.get("nrows")
+    kwargs["nrows"] = nrows if limit is None else min(int(limit), nrows)
+    try:
+        sample = pd.read_excel(path, dtype=str, **kwargs)
+    except (OSError, ValueError, KeyError, ImportError):
+        return {}
+    if isinstance(sample, dict):  # defensive: sheet_name resolved to many
+        return {}
+
+    padded: dict[Hashable, type[str]] = {}
+    for position, column in enumerate(sample.columns):
+        values = sample.iloc[:, position].dropna()
+        if values.empty or not _has_leading_zero_ids(values):
+            continue
+        if safe_to_numeric(values, errors="coerce").notna().all():
+            padded[column] = str
+    return padded

src/freshdata/api.py

@@ -9,7 +9,7 @@
 
 import pandas as pd
 
-from ._csv_io import leading_zero_dtypes
+from ._csv_io import leading_zero_dtypes, leading_zero_dtypes_excel
 from ._reportframe import ReportFrame
 from ._util import require_unique_labels, sanitize_csv_formulas
 from .adapters.polars import from_pandas, to_pandas
@@ -620,7 +620,12 @@ def clean_excel(
     """
     if "report" in options:
         return_report = bool(options.pop("report"))
-    df = pd.read_excel(path, **(read_excel_kwargs or {}))
+    excel_kwargs = dict(read_excel_kwargs or {})
+    if _preserve_leading_zeros(config, options):
+        padded = leading_zero_dtypes_excel(path, read_excel_kwargs=excel_kwargs)
+        if padded:
+            excel_kwargs["dtype"] = padded
+    df = pd.read_excel(path, **excel_kwargs)
     if isinstance(df, dict):
         raise TypeError(
             "clean_excel cleans a single sheet; pass "

tests/test_excel_leading_zeros.py

@@ -0,0 +1,121 @@
+"""``clean_excel`` must preserve zero padding, as ``clean_csv`` already does.
+
+``pandas.read_excel`` infers types exactly as ``read_csv`` does, so a cell that
+openpyxl stored as the *text* ``"02134"`` arrived as the integer ``2134`` and
+the padding was gone before any cleaning step ran. ``clean_csv`` avoids this
+with the ``_csv_io.leading_zero_dtypes`` pre-scan; ``clean_excel`` called
+``pd.read_excel`` directly and had no equivalent, so
+``preserve_leading_zeros=True`` -- documented as a shared option -- changed
+nothing there.
+
+The loss was silent: no warning, no report entry, and a postcode column read as
+integers then goes on to be profiled and outlier-checked as a quantity.
+"""
+
+from __future__ import annotations
+
+import openpyxl
+import pandas as pd
+import pytest
+
+import freshdata as fd
+
+ZIPS = ["02134", "10001", "94105", "00501", "07030"]
+
+
+def _workbook(tmp_path, zips=ZIPS, *, sheet="Sheet1"):
+    """Write genuine TEXT cells, so the defect cannot be blamed on the file."""
+    path = tmp_path / "zips.xlsx"
+    book = openpyxl.Workbook()
+    sheet_obj = book.active
+    sheet_obj.title = sheet
+    sheet_obj.append(["cust", "zip", "qty"])
+    for row, zip_code in enumerate(zips, start=1):
+        sheet_obj.cell(row=row + 1, column=1, value=f"c{row}")
+        cell = sheet_obj.cell(row=row + 1, column=2)
+        cell.value = zip_code
+        cell.data_type = "s"
+        sheet_obj.cell(row=row + 1, column=3, value=row * 10)
+    book.save(path)
+    return path
+
+
+def test_the_workbook_really_stores_text(tmp_path):
+    """Guard the guard: if the fixture stored numbers, the rest proves nothing."""
+    loaded = openpyxl.load_workbook(_workbook(tmp_path))
+    assert loaded.active["B2"].value == "02134"
+    assert loaded.active["B2"].data_type == "s"
+
+
+def test_leading_zeros_survive_clean_excel(tmp_path):
+    out = fd.clean_excel(_workbook(tmp_path), verbose=False)
+    assert out["zip"].tolist() == ZIPS
+    assert out["zip"].dtype == object
+
+
+def test_only_the_padded_column_is_forced_to_text(tmp_path):
+    """A genuine quantity column must keep its numeric dtype."""
+    out = fd.clean_excel(_workbook(tmp_path), verbose=False)
+    assert pd.api.types.is_integer_dtype(out["qty"])
+
+
+def test_the_csv_and_excel_paths_now_agree(tmp_path):
+    """The two entry points are documented as companions; they must match."""
+    csv_path = tmp_path / "zips.csv"
+    pd.DataFrame({"cust": [f"c{i}" for i in range(1, 6)], "zip": ZIPS}).to_csv(
+        csv_path, index=False
+    )
+    from_csv = fd.clean_csv(csv_path, verbose=False)
+    from_excel = fd.clean_excel(_workbook(tmp_path), verbose=False)
+    assert from_csv["zip"].tolist() == from_excel["zip"].tolist() == ZIPS
+
+
+def test_a_csv_to_excel_hand_off_keeps_the_padding(tmp_path):
+    """The end-to-end shape that lost data: clean CSV, store as xlsx, clean again."""
+    csv_path = tmp_path / "zips.csv"
+    xlsx_path = tmp_path / "mid.xlsx"
+    pd.DataFrame({"cust": [f"c{i}" for i in range(1, 6)], "zip": ZIPS}).to_csv(
+        csv_path, index=False
+    )
+    cleaned = fd.clean_csv(csv_path, verbose=False)
+    pd.DataFrame(cleaned).to_excel(xlsx_path, index=False)
+    assert fd.clean_excel(xlsx_path, verbose=False)["zip"].tolist() == ZIPS
+
+
+def test_preserve_leading_zeros_false_still_opts_out(tmp_path):
+    """The option must remain an option, not become unconditional behaviour."""
+    out = fd.clean_excel(_workbook(tmp_path), verbose=False, preserve_leading_zeros=False)
+    assert pd.api.types.is_integer_dtype(out["zip"])
+    assert out["zip"].iloc[0] == 2134
+
+
+def test_an_explicit_dtype_still_wins(tmp_path):
+    """The caller decides types when they say so; the pre-scan must stand down."""
+    out = fd.clean_excel(
+        _workbook(tmp_path), verbose=False, read_excel_kwargs={"dtype": {"zip": str}}
+    )
+    assert out["zip"].tolist() == ZIPS
+
+
+def test_a_column_without_padding_is_untouched(tmp_path):
+    """No false positives: ordinary numbers must not be turned into text."""
+    out = fd.clean_excel(
+        _workbook(tmp_path, zips=["12345", "23456", "34567", "45678", "56789"]),
+        verbose=False,
+    )
+    assert pd.api.types.is_integer_dtype(out["zip"])
+
+
+def test_a_named_sheet_is_pre_scanned_too(tmp_path):
+    """The pre-scan must follow sheet_name, or it samples the wrong sheet."""
+    path = _workbook(tmp_path, sheet="Q3")
+    out = fd.clean_excel(path, verbose=False, read_excel_kwargs={"sheet_name": "Q3"})
+    assert out["zip"].tolist() == ZIPS
+
+
+def test_selecting_several_sheets_still_raises_the_documented_error(tmp_path):
+    """The pre-scan must not mask clean_excel's own multi-sheet rejection."""
+    with pytest.raises(TypeError, match="cleans a single sheet"):
+        fd.clean_excel(
+            _workbook(tmp_path), verbose=False, read_excel_kwargs={"sheet_name": None}
+        )