From 6cb7441c7fac2fb5dd8920addef8089c53a8f538 Mon Sep 17 00:00:00 2001 From: slegarraga Date: Fri, 21 Aug 2026 19:30:37 -0400 Subject: [PATCH 1/3] fix: CRMCleaner no longer corrupts complex amounts into finite floats A single np.complex128 cell (Excel/openpyxl formula round-trip) survived the transform-time object-cast retry and reached the currency parser's string path, where str(3+4j) == "(3+4j)" matched the parenthesised- negative rule and produced a plausible but wrong -34.0 (#129). Two layers: - element-wise isinstance(v, complex) guard on object columns masks the cells to NaN with a warning before any string cleanup; - had_value is now computed before masking so an all-unparseable column still raises per the documented contract. Verified: repro from #129 now yields NaN + UserWarning; full suite 1843 passed / 23 skipped. Signed-off-by: slegarraga --- philanthropy/preprocessing/_transformers.py | 19 ++++++++++++ tests/test_preprocessing.py | 33 +++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/philanthropy/preprocessing/_transformers.py b/philanthropy/preprocessing/_transformers.py index 5d9a1b8..8850b85 100755 --- a/philanthropy/preprocessing/_transformers.py +++ b/philanthropy/preprocessing/_transformers.py @@ -17,6 +17,8 @@ from typing import Any +import warnings + import numpy as np import pandas as pd @@ -59,7 +61,24 @@ def _coerce_currency_to_float(col: pd.Series) -> pd.Series: if pd.api.types.is_numeric_dtype(col): return pd.to_numeric(col, errors="coerce").astype("float64") + # Complex values (e.g. np.complex128 cells that survive an object-cast + # retry, as when a formula column round-trips through Excel/openpyxl) + # must never reach the string path: str(3+4j) is "(3+4j)", which matches + # the parenthesised-negative rule and corrupts the cell into -34.0 + # (#129). NaN + warning, consistent with "values that still don't parse + # become NaN". had_value = col.notna() + + if col.dtype == object: + complex_mask = col.map(lambda v: isinstance(v, complex)).fillna(False) + if complex_mask.any(): + warnings.warn( + f"CRMCleaner: {int(complex_mask.sum())} complex value(s) in " + f"amount column {col.name!r} cannot be parsed as currency; " + f"they became NaN.", + stacklevel=2, + ) + col = col.mask(complex_mask) cleaned = col.astype(str).str.strip() cleaned = cleaned.str.replace(r"^\((.*)\)$", r"-\1", regex=True) cleaned = cleaned.str.replace(r"[^0-9eE.\-]", "", regex=True) diff --git a/tests/test_preprocessing.py b/tests/test_preprocessing.py index 1164471..67acc77 100755 --- a/tests/test_preprocessing.py +++ b/tests/test_preprocessing.py @@ -96,6 +96,39 @@ def test_amount_col_unparseable_column_raises(self): with pytest.raises(ValueError, match="could not parse"): cleaner.fit_transform(df) + def test_complex_amount_becomes_nan_with_warning_not_corruption(self): + # #129: a single np.complex128 cell (Excel/openpyxl formula column + # round-trip) used to slip through the object-cast retry and corrupt + # into a plausible finite float — str(3+4j) == "(3+4j)" matched the + # parenthesised-negative rule, yielding -34.0. Contract now: in a + # column with other parseable values, the complex cell becomes NaN + # with a warning, consistent with "values that still don't parse + # become NaN". + X_fit = pd.DataFrame({ + "gift_date": ["2023-01-01", "2023-02-01"], + "gift_amount": [5.0, 10.0], + }) + cleaner = CRMCleaner().set_output(transform="pandas").fit(X_fit) + X_bad = pd.DataFrame({ + "gift_date": ["2023-03-01", "2023-04-01"], + "gift_amount": pd.Series([np.complex128(3 + 4j), 25.0], dtype=object), + }) + with pytest.warns(UserWarning, match="complex value"): + out = cleaner.transform(X_bad) + assert np.isnan(out["gift_amount"].iloc[0]) + assert out["gift_amount"].iloc[1] == 25.0 + + def test_complex_whole_column_still_raises(self): + # A column where nothing parses still raises, complex cells included: + # masking must not swallow the all-unparseable contract. + df = pd.DataFrame({ + "gift_date": ["2023-03-01"], + "gift_amount": pd.Series([np.complex128(3 + 4j)], dtype=object), + }) + cleaner = CRMCleaner() + with pytest.raises(ValueError, match="could not parse"): + cleaner.fit_transform(df) + def test_date_col_coerced_to_datetime(self): df = pd.DataFrame({"gift_date": ["2023-07-01"], "gift_amount": [100.0]}) cleaner = CRMCleaner().set_output(transform="pandas") From 384f2ba6e8342552dbe7274f95046ef64cd76a02 Mon Sep 17 00:00:00 2001 From: slegarraga Date: Sat, 22 Aug 2026 12:37:58 -0400 Subject: [PATCH 2/3] docs(changelog): add Unreleased entry for the complex-amount fix Per AGENTS.md, every PR touching philanthropy/ needs an entry under [Unreleased]. Describes the two-layer guard from this branch: element-wise complex masking to NaN with a warning, and preserved raise-on-nothing-parses contract. --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4320b0f..d3056d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,13 @@ Format: [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) ## [Unreleased] +### Fixed +- `CRMCleaner.transform` no longer silently corrupts complex amounts into + wrong finite floats: cells holding actual `complex` values are masked to + NaN with a `UserWarning` naming them, and a column where nothing parses + (all-complex included) still raises `could not parse` per the documented + contract. Closes #129. + ## [1.0.0] - TBD The API freeze. No code changes: 1.0.0 is a promise, not a feature. From b939ff5001766ed5d95907fa409a8902ff853266 Mon Sep 17 00:00:00 2001 From: slegarraga Date: Sat, 22 Aug 2026 13:23:11 -0400 Subject: [PATCH 3/3] chore: retrigger CI on final head Signed-off-by: slegarraga