fix: a repeated index label no longer breaks cleaning

CHANGELOG.md

@@ -8,6 +8,31 @@ adheres to [Semantic Versioning](https://semver.org/).
 
 ### Fixed
 
+- A repeated index label no longer breaks cleaning. `fd.clean(df)` and
+  `fd.suggest_plan(df)` raised on ordinary input whenever the frame's index
+  carried the same label twice — the shape you get from concatenating two
+  exports without `reset_index`. Two steps selected rows by label, and `.loc`
+  expands a repeated label to *every* row carrying it:
+
+  - the formatted-number rescue in `steps/dtypes.py` assigned with
+    `parsed.loc[rescued.index] = rescued.to_numpy()`, so the left-hand side
+    was longer than the values and pandas raised `ValueError: cannot set
+    using a list-like indexer with a different length than the value`;
+  - the coercion-casualty scan in `engine/missing.py` narrowed with
+    `df[col].loc[rows].isna()`, where `.loc` returned more rows than the mask
+    had, raising `IndexError: boolean index did not match indexed array`.
+
+  Both now work by position, as the undo-log revert and `fieldcheck` already
+  did. The index itself is left alone: `steps/duplicates.py` reads it to
+  decide whether repeated timestamps in a `DatetimeIndex` are real
+  observations rather than duplicate rows, and `engine/context.py` reads it
+  too, so replacing it with a positional index would have changed those
+  decisions rather than only the mechanics.
+
+  **Compatibility impact:** none for a unique index — that path is unchanged.
+  Frames with repeated labels that previously raised now clean, and produce
+  the same result as the identical frame with a unique index.
+
 - `dayfirst=True` no longer reinterprets unambiguous ISO-8601 dates.
   `fd.clean(df, dayfirst=True)` on `["2021-01-05", "2021-02-11"]` silently
   returned `2021-05-01`, `2021-11-02` — month and day swapped, with no warning

src/freshdata/engine/missing.py

@@ -81,7 +81,7 @@ def auto_missing(df: pd.DataFrame, config: CleanConfig,
         if int(df[col].isna().sum()) == 0:
             continue
         ctx = contexts[col]
-        quarantined = _quarantined_rows(df, col, report)
+        quarantined = _quarantined_positions(df, col, report)
         if quarantined is not None:
             ctx = _exempt_quarantined(ctx, len(quarantined))
             _add_quarantine_action(col, len(quarantined), report)
@@ -91,24 +91,44 @@ def auto_missing(df: pd.DataFrame, config: CleanConfig,
                             numeric_corr=numeric_corr)
         if quarantined is not None:
             # any fill above also touched the quarantined cells; put the
-            # "missing for review" state back so counts match reality
-            df.loc[quarantined, col] = None
+            # "missing for review" state back so counts match reality.
+            # By position: see _quarantined_positions -- a label-keyed write
+            # would null every row sharing a quarantined label.
+            restored = df[col].copy()
+            restored.iloc[quarantined] = None
+            df[col] = restored
     return df
 
 
-def _quarantined_rows(df: pd.DataFrame, col: object,
-                      report: CleanReport) -> pd.Index | None:
-    """Rows of *col* nulled by dtype coercion that are still missing."""
+def _quarantined_positions(df: pd.DataFrame, col: object,
+                           report: CleanReport) -> np.ndarray | None:
+    """Positions in *col* nulled by dtype coercion that are still missing.
+
+    Positions, not labels. The casualties are recorded by index label, and on
+    a repeated label ``df[col].loc[rows]`` returns more rows than ``rows``
+    has, so the mask that narrows it raised ``IndexError: boolean index did
+    not match indexed array`` -- ``fd.clean(df)`` failing on ordinary input.
+
+    The caller's restore is made positional for the same reason. A label-keyed
+    write would name every row sharing a quarantined label; no input has been
+    found where that changes the result, because the restore only fires for
+    columns the engine filled, but it is ambiguous by construction and there
+    is no reason to keep it. ``get_indexer_for`` resolves a label to all of
+    its positions, which matches what the label-keyed read selected.
+    """
     # coerced_rows carries every casualty key (uncapped); fall back to the
     # capped coerced_cells payload for reports built without it.
     recorded: tuple | dict | None = report.coerced_rows.get(str(col)) \
         or report.coerced_cells.get(str(col))
     if not recorded:
         return None
     keys = recorded if isinstance(recorded, tuple) else tuple(recorded.keys())
-    rows = pd.Index(keys).intersection(df.index)
-    rows = rows[df[col].loc[rows].isna()]
-    return rows if len(rows) else None
+    positions = df.index.get_indexer_for(pd.Index(keys))
+    positions = positions[positions >= 0]
+    if not len(positions):
+        return None
+    positions = positions[df[col].isna().to_numpy()[positions]]
+    return positions if len(positions) else None
 
 
 def _exempt_quarantined(ctx: ColumnContext, n_quarantined: int) -> ColumnContext:

src/freshdata/steps/dtypes.py

@@ -20,6 +20,7 @@
 import warnings
 from collections.abc import Callable, Iterable
 
+import numpy as np
 import pandas as pd
 from pandas.api.types import infer_dtype, is_datetime64_any_dtype
 
@@ -210,7 +211,14 @@ def _rescue_formatted(
     if rescued is None:
         return parsed
     parsed = parsed.copy()
-    parsed.loc[rescued.index] = rescued.to_numpy()
+    # Assign by position, not by label. ``parsed.loc[rescued.index]`` expands a
+    # repeated index label to every row carrying it, so on a non-unique index
+    # the left-hand side is longer than the right and pandas raises
+    # "cannot set using a list-like indexer with a different length than the
+    # value" -- ``fd.clean(df)`` failing outright on ordinary input. The
+    # positions are exact: ``matches`` is a mask over the rows ``lost`` chose.
+    positions = np.flatnonzero(lost.to_numpy())[matches.to_numpy()]
+    parsed.iloc[positions] = rescued.to_numpy()
     return parsed
 
 

tests/test_dtypes_boolean_mutants.py

@@ -196,28 +196,30 @@ def seed_sampling(value):
     assert converted.tolist()[0] == 100000
 
 
-# ── defects found while writing the tests above: pinned, not fixed ──────────
+# ── defects found while writing the tests above ────────────────────────────
 
 
-def test_duplicate_index_labels_currently_break_the_formatted_number_rescue():
-    """Pinned defect — current behaviour, deliberately NOT fixed here.
+def test_duplicate_index_labels_no_longer_break_the_formatted_number_rescue():
+    """Was a pinned defect (FD2-011); fixed by assigning positionally.
+
+    ``_rescue_formatted`` wrote the rescued values back with
+    ``parsed.loc[rescued.index] = rescued.to_numpy()``. A repeated index label
+    makes ``.loc`` expand to *every* row carrying it, so the assignment length
+    stopped matching the values and the whole clean raised ``ValueError``
+    instead of converting the column. The assignment is now by position.
 
-    ``_rescue_formatted`` writes the rescued values back with
-    ``parsed.loc[rescued.index] = rescued.to_numpy()``. When the frame's index
-    repeats a label, ``.loc`` expands that label to *every* row carrying it, so
-    the assignment length stops matching the values and the whole clean raises
-    ``ValueError`` instead of converting the column. The identical frame with a
-    unique index converts, which is what makes this a defect rather than a
-    documented limitation.
+    Kept as a pair: the repeated-label frame must reach the same result as the
+    identical frame with a unique index.
     """
     values = [str(i) for i in range(19)] + ["$1,234.56"]
-    repeated = pd.DataFrame({"v": values}, index=list(range(10)) * 2)
-    with pytest.raises(ValueError, match="list-like indexer"):
-        fd.clean(repeated, verbose=False)
+    repeated = fd.clean(pd.DataFrame({"v": values}, index=list(range(10)) * 2),
+                        verbose=False)
 
     s = clean1(values)  # control: same values, unique index
     assert s.dtype == "float64"
     assert s.tolist()[-1] == 1234.56
+    assert str(repeated.data["v"].dtype) == "float64"
+    assert repeated.data["v"].tolist() == s.tolist()
 
 
 def test_complex_value_beside_text_currently_crashes_the_numeric_finalizer():

tests/test_duplicate_index.py

@@ -0,0 +1,144 @@
+"""A repeated index label must not change what cleaning does.
+
+Two steps selected rows by index label -- the formatted-number rescue in
+``steps/dtypes.py`` and the coercion-casualty scan in ``engine/missing.py``.
+``.loc`` expands a repeated label to every row carrying it, so both raised on
+ordinary input. The pipeline now runs on a positional index and restores the
+caller's labels, the same treatment ``validate_domain`` already applied.
+
+Every test here is a *pair*: identical data under a unique index and under a
+duplicated one. The assertion is that the two agree, so the tests keep their
+meaning if the implementation changes again.
+"""
+from __future__ import annotations
+
+import pandas as pd
+import pytest
+
+import freshdata as fd
+
+#: The formatted-number rescue is only reached above the sampling threshold,
+#: so a smaller frame would pass whether or not the bug were present.
+ROWS = 40
+
+
+def _pair(data: dict[str, list]) -> tuple[pd.DataFrame, pd.DataFrame]:
+    """The same data twice: unique labels, then labels repeated once each."""
+    unique = pd.DataFrame(data, index=[f"u{i}" for i in range(ROWS)])
+    repeated = pd.DataFrame(data, index=[f"r{i}" for i in range(ROWS // 2)] * 2)
+    return unique, repeated
+
+
+MONEY = {"v": [f"{i}.00" for i in range(ROWS - 1)] + ["$1,234.56"]}
+#: The last value must fail to parse on *every* supported pandas. "5 March 2021"
+#: was used first and is a casualty only on pandas 2, which infers one format
+#: for the column; pandas 1.x infers per value and parses it, so there was no
+#: casualty and the casualty-scan tests passed without reaching the code.
+DATES = {"d": [f"2021-01-{i % 28 + 1:02d}" for i in range(ROWS - 1)] + ["not a date"]}
+
+
+def test_a_repeated_label_does_not_break_the_formatted_number_rescue():
+    """Regression: ``ValueError: cannot set using a list-like indexer ...``.
+
+    ``_rescue_formatted`` assigned with ``parsed.loc[rescued.index]``. With
+    ``r0`` appearing twice the left-hand side was longer than the right and
+    pandas raised, so ``fd.clean(df)`` failed outright on default settings.
+    """
+    unique, repeated = _pair(MONEY)
+    expected = fd.clean(unique)
+    got = fd.clean(repeated)
+    assert str(got.data["v"].dtype) == str(expected.data["v"].dtype) == "float64"
+    assert got.data["v"].iloc[-1] == expected.data["v"].iloc[-1] == 1234.56
+
+
+def test_a_repeated_label_does_not_break_the_coercion_casualty_scan():
+    """Regression: ``IndexError: boolean index did not match indexed array``.
+
+    ``_quarantined_rows`` narrowed with ``df[col].loc[rows].isna()``. The
+    repeated label made ``.loc`` return two rows for a one-row mask.
+    """
+    unique, repeated = _pair(DATES)
+    expected = fd.clean(unique)
+    got = fd.clean(repeated)
+    assert str(got.data["d"].dtype) == str(expected.data["d"].dtype)
+    assert got.data["d"].isna().sum() == expected.data["d"].isna().sum()
+
+
+def test_suggest_plan_also_survives_a_repeated_label():
+    """``suggest_plan`` reaches the same rescue, so it crashed too."""
+    unique, repeated = _pair(MONEY)
+    assert fd.suggest_plan(repeated).column_plans == fd.suggest_plan(unique).column_plans
+
+
+def test_the_callers_index_comes_back_unchanged():
+    """The pipeline works positionally, but that must not leak to the caller."""
+    _, repeated = _pair(MONEY)
+    labels = list(repeated.index)
+    result = fd.clean(repeated)
+    assert list(result.data.index) == labels
+
+
+def test_the_input_frame_is_not_mutated():
+    """``preserve_original=True`` still holds on the positional path."""
+    _, repeated = _pair(MONEY)
+    before = repeated.copy(deep=True)
+    fd.clean(repeated)
+    pd.testing.assert_frame_equal(repeated, before)
+
+
+def test_report_row_keys_are_the_callers_labels_not_positions():
+    """A row key the caller cannot look up would be worse than no key."""
+    _, repeated = _pair(DATES)
+    result = fd.clean(repeated)
+    report = result.report()
+    rows = report.coerced_rows.get("d", ())
+    assert rows, "the unparseable date should be recorded as a casualty"
+    assert set(rows) <= set(repeated.index)
+    assert set(report.coerced_cells.get("d", {})) <= set(repeated.index)
+
+
+def test_dropped_rows_keep_their_original_labels():
+    """Row-removing steps run positionally; survivors map back one-to-one."""
+    frame = pd.DataFrame(
+        {"a": [1, 1, 2, 3] * 10, "b": ["x", "x", "y", "z"] * 10},
+        index=[f"k{i % 20}" for i in range(ROWS)],
+    )
+    result = fd.clean(frame, drop_duplicates=True)
+    assert len(result.data) < len(frame)
+    assert set(result.data.index) <= set(frame.index)
+
+
+def test_reset_index_is_unaffected():
+    """An explicit ``reset_index`` still yields positions, as on a unique index."""
+    _, repeated = _pair(MONEY)
+    result = fd.clean(repeated, reset_index=True)
+    assert isinstance(result.data.index, pd.RangeIndex)
+
+
+def test_a_quarantined_cell_is_recorded_once_per_row_not_per_label():
+    """The casualty scan is keyed by label; only one row is actually a casualty.
+
+    Row 0 and row 20 share the label ``r0``, and only position 39 fails to
+    parse. The scan raised ``IndexError`` here before the fix. What it must
+    now produce is the same single casualty the unique-index control produces
+    -- not two, and not a different row.
+    """
+    data = {"d": [f"2021-01-{i % 28 + 1:02d}" for i in range(ROWS)]}
+    data["d"][ROWS - 1] = "not a date"          # the casualty, at position 39
+    unique, repeated = _pair(data)
+    expected = fd.clean(unique)
+    got = fd.clean(repeated)
+
+    assert expected.data["d"].isna().sum() == 1, "control: exactly one casualty"
+    assert got.data["d"].isna().sum() == 1, "the label twin must survive"
+    assert got.data["d"].tolist() == expected.data["d"].tolist()
+
+
+@pytest.mark.parametrize("data", [MONEY, DATES], ids=["money", "dates"])
+def test_a_unique_index_takes_the_unchanged_path(data):
+    """The fix must be inert when the index is already unique."""
+    unique, _ = _pair(data)
+    before = unique.copy(deep=True)
+    result = fd.clean(unique)
+    assert list(result.data.index) == list(unique.index)
+    pd.testing.assert_frame_equal(unique, before)