Found while writing the coverage tests for #118 (this is the observation I flagged in that PR body). Reproducible on current main; filing as its own issue per the request in #126 for genuine external-user signal — this is a real friction point I hit, not manufactured.
What happens
A DataFrame whose amount column mixes regular values with one Python complex slips through every guard in CRMCleaner.transform and produces a plausible-looking but wrong finite number, instead of raising or NaN-ing:
import numpy as np
import pandas as pd
from philanthropy.preprocessing import CRMCleaner
X_fit = pd.DataFrame({
"gift_date": ["2023-01-01", "2023-02-01"],
"gift_amount": [5.0, 10.0],
})
t = CRMCleaner().fit(X_fit)
X_bad = pd.DataFrame({
"gift_date": ["2023-03-01"],
"gift_amount": pd.Series([np.complex128(3 + 4j)], dtype=object),
})
print(t.transform(X_bad))
# gift_date 2023-03-01 00:00:00
# gift_amount -34.0 <- silently corrupted3+4j becomes -34.0. For context on how easy this is to hit: np.complex128 values show up in real CRM exports when a formula column round-trips through Excel/openpyxl.
Root cause (three guards miss it in sequence)
- The mixed str/complex frame raises
DTypePromotionError in the first validate_data pass; the object-cast retry introduced for exactly this kind of input lets it through.
np.iscomplexobj(X_arr) returns False because after the retry the array has object dtype — iscomplexobj inspects dtype, not elements._coerce_currency_to_float takes the string path: str(3+4j) is "(3+4j)", which matches the parenthesised-negative rule (-\1), then character-stripping deletes the parens and j, leaving "-34" → -34.0. Sign flipped, imaginary part dropped, digits concatenated.
The same frame passed directly to validate_data raises Complex data not supported cleanly at fit time — the corruption is specific to the transform-time retry path.
Suggested direction
In _coerce_currency_to_float's string path, reject parsed-out-of-nothing values that were originally complex before the regex cleanup, or check element-wise isinstance(v, complex) on object columns before casting to str. A regression test would assert either a raise or NaN for the complex cell — anything but a finite number. Happy to send the PR if you tell me which behavior you want (raise vs NaN); my instinct is NaN-plus-warning, consistent with "values that still don't parse become NaN".
Found while writing the coverage tests for #118 (this is the observation I flagged in that PR body). Reproducible on current main; filing as its own issue per the request in #126 for genuine external-user signal — this is a real friction point I hit, not manufactured.
What happens
A DataFrame whose amount column mixes regular values with one Python
complexslips through every guard inCRMCleaner.transformand produces a plausible-looking but wrong finite number, instead of raising or NaN-ing:3+4jbecomes -34.0. For context on how easy this is to hit:np.complex128values show up in real CRM exports when a formula column round-trips through Excel/openpyxl.Root cause (three guards miss it in sequence)
DTypePromotionErrorin the firstvalidate_datapass; the object-cast retry introduced for exactly this kind of input lets it through.np.iscomplexobj(X_arr)returns False because after the retry the array has object dtype —iscomplexobjinspects dtype, not elements._coerce_currency_to_floattakes the string path:str(3+4j)is"(3+4j)", which matches the parenthesised-negative rule (-\1), then character-stripping deletes the parens andj, leaving"-34"→-34.0. Sign flipped, imaginary part dropped, digits concatenated.The same frame passed directly to
validate_dataraisesComplex data not supportedcleanly at fit time — the corruption is specific to the transform-time retry path.Suggested direction
In
_coerce_currency_to_float's string path, reject parsed-out-of-nothing values that were originally complex before the regex cleanup, or check element-wiseisinstance(v, complex)on object columns before casting to str. A regression test would assert either a raise or NaN for the complex cell — anything but a finite number. Happy to send the PR if you tell me which behavior you want (raise vs NaN); my instinct is NaN-plus-warning, consistent with "values that still don't parse become NaN".