Skip to content

Commit fce9046

Browse files
fix(dtypes): a complex value in an object column no longer crashes cleaning (#507)
fd.clean(pd.DataFrame({"v": [complex(1, 2), "abc", "3"]})) raised TypeError from the numeric finalizer's integrality check (nonnull % 1 == 0), and so did a complex value among numeric strings or Python numbers (complex, numpy.complex128, numpy.complex64). Cause: once one cell is complex, pd.to_numeric(errors="coerce") returns a complex buffer and never writes the text, bytes or bool cells into it -- they come back as uninitialised memory, whatever the last freed buffer of that size held. Reproduced on pandas 2.3.3 and 1.5.3: after freeing a buffer of 7+7j, to_numeric([1+2j, "abc", "not a number"]) returns [(1+2j), (7+7j), (7+7j)]. So the column passed or failed numeric_threshold depending on what had run earlier in the process, and when it passed the finalizer raised. This was the unexplained order dependence behind a flaky characterization test. Every numeric target in steps/dtypes.py is a real dtype, so a complex value is now an unparseable straggler like any other: when pandas returns a complex result, complex cells are masked *before* re-parsing (the garbage cannot be filtered out afterwards), and the existing threshold, coerced_cells and type-contamination paths handle it. Columns without a complex value never take the new path. tests/test_exotic_values_in_numeric_columns.py: 22 tests, two of which seed the freed buffer so the garbage is deterministic. With the converted characterization tests, 17 fail without this change on both py3.12/pandas 2.3.3 and py3.9/pandas 1.5.3.
1 parent 28ce1c3 commit fce9046

4 files changed

Lines changed: 316 additions & 43 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,33 @@ adheres to [Semantic Versioning](https://semver.org/).
8686
the `dayfirst="auto"` and `semantic_context` routes, which were never
8787
affected.
8888

89+
- A complex number in an object column no longer crashes cleaning.
90+
`fd.clean(pd.DataFrame({"v": [complex(1, 2), "abc", "3"]}))` raised
91+
`TypeError: ufunc 'remainder' not supported for the input types` from the
92+
numeric finalizer's integrality check (`nonnull % 1 == 0`), and so did a
93+
complex value among numeric strings or among Python numbers in an object
94+
column (Python `complex`, `numpy.complex128` and `numpy.complex64` alike).
95+
Cause: once one cell is complex, `pd.to_numeric` returns a complex result,
96+
and it never writes a text, bytes or bool cell into that result — those
97+
cells come back as whatever numpy's last freed buffer of the same size held.
98+
`"abc"` and `"3"` could read as `0j`, `7+7j` or NaN, so the column passed or
99+
failed `numeric_threshold` depending on what had run earlier in the process,
100+
and when it passed the finalizer raised. Every numeric target in
101+
`steps/dtypes.py` is a real dtype, so a complex value is now treated like
102+
any other value the parse cannot interpret: it is masked before parsing, it
103+
counts against `numeric_threshold`, and it is never mangled into a real
104+
number. When the column still converts, the complex cell is set to missing
105+
with its original in `report.coerced_cells`, like a word or a `Fraction` in
106+
the same position; when it does not, the column is returned untouched and
107+
the type-contamination warning names the row.
108+
109+
**Compatibility impact:** only columns holding a complex value change, and
110+
every such column previously raised (or, depending on process state, came
111+
back unchanged). Columns without a complex value parse exactly as before —
112+
the extra step runs only when pandas returns a complex result. A column made
113+
entirely of complex values is still declined before parsing and keeps its
114+
dtype.
115+
89116

90117
### Documentation
91118

‎src/freshdata/steps/dtypes.py‎

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -182,13 +182,36 @@ def _try_boolean(s: pd.Series, nonnull: pd.Series) -> pd.Series | None:
182182
return converted
183183

184184

185+
def _is_complex_cell(value: object) -> bool:
186+
return isinstance(value, (complex, np.complexfloating))
187+
188+
185189
def _to_numeric_or_none(values: pd.Series) -> pd.Series | None:
186190
"""``to_numeric`` that tolerates non-scalar cells (lists raise even with
187-
``errors="coerce"``)."""
191+
``errors="coerce"``) and treats a complex cell as unparseable."""
188192
# safe_to_numeric masks cells whose leading exponent can overflow the C int
189193
# in pandas < 3's parser; every other cell is parsed exactly as pandas does.
190194
try:
191-
return safe_to_numeric(values, errors="coerce")
195+
parsed = safe_to_numeric(values, errors="coerce")
196+
except (TypeError, ValueError):
197+
return None
198+
if parsed.dtype.kind != "c":
199+
return parsed
200+
# pandas returns a complex result only when a cell is complex. Every target
201+
# here is a *real* dtype, so a complex value is not a number this step can
202+
# hold: it is an unparseable straggler, like any other non-numeric cell --
203+
# counted against the threshold, quarantined in coerced_cells if the column
204+
# converts, left untouched if it does not. It must be masked *before*
205+
# parsing, not filtered out of the complex result: pandas never writes a
206+
# text, bytes or bool cell into its complex buffer, so beside a complex
207+
# value those cells come back as uninitialised memory ("abc" and "3" as 0j,
208+
# or as arbitrary values that differ between identical calls) -- counted as
209+
# parsed, then crashing the real-number finalizer.
210+
complex_cells = np.fromiter(
211+
map(_is_complex_cell, values.to_numpy(dtype=object)), dtype=bool, count=len(values)
212+
)
213+
try:
214+
return safe_to_numeric(values.astype(object).mask(complex_cells), errors="coerce")
192215
except (TypeError, ValueError):
193216
return None
194217

‎tests/test_dtypes_boolean_mutants.py‎

Lines changed: 39 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -222,58 +222,56 @@ def test_duplicate_index_labels_no_longer_break_the_formatted_number_rescue():
222222
assert repeated.data["v"].tolist() == s.tolist()
223223

224224

225-
def test_complex_value_beside_text_currently_crashes_the_numeric_finalizer():
226-
"""Pinned defect — current behaviour, deliberately NOT fixed here.
227-
228-
``_finalize_numeric``'s integrality check is ``nonnull % 1 == 0``, and
229-
``complex`` has no ``%``, so a complex value that reaches the finalizer
230-
raises ``TypeError`` instead of the column being left alone. A column of
231-
complex values *alone* is declined earlier (``infer_dtype`` reports
232-
``"complex"``, which is not a text-ish kind); one complex value beside text
233-
makes the column ``"mixed"``, which is how it gets there.
234-
235-
Asserted against ``_finalize_numeric`` directly rather than through
236-
``fd.clean``. The end-to-end route is **order-dependent**: the same
237-
``fd.clean(...)`` call raises in a fresh process but does not after certain
238-
other work has happened in the same process, so an ``fd.clean``-level
239-
``pytest.raises`` here was genuinely flaky under randomised test ordering.
240-
The unit-level behaviour is stable, and it is the defect. See
241-
``test_the_end_to_end_route_to_this_crash_is_order_dependent``.
225+
def test_complex_value_beside_text_no_longer_crashes_the_numeric_finalizer():
226+
"""Formerly a pinned defect (FD2-012), now fixed.
227+
228+
This test used to be ``..._currently_crashes_...`` and asserted that
229+
``_finalize_numeric``'s integrality check (``nonnull % 1 == 0``) raised
230+
``TypeError`` on a complex value, which one complex value beside text
231+
reached through ``fd.clean``. The finalizer itself is unchanged and still
232+
cannot take a complex value; what changed is that nothing hands it one.
233+
``_to_numeric_or_none`` treats a complex cell as unparseable (every target
234+
here is a real dtype), so the column is judged by its real values: one of
235+
three parses, the column is declined, and it comes back untouched.
236+
237+
The route used to be order-dependent (it raised in a fresh process but not
238+
after some other work). The cause was pandas: once a cell is complex,
239+
``to_numeric`` returns its complex buffer without ever writing the text
240+
cells into it, so ``"abc"`` and ``"3"`` came back as whatever numpy's last
241+
freed buffer of that size held -- NaN (column declined, no crash) or a
242+
number (column "parsed", then the crash). See
243+
``tests/test_exotic_values_in_numeric_columns.py``.
242244
"""
243-
# Three spellings of the same failure across pandas/numpy versions: object
244-
# dtype falls back to Python's ``%`` ("unsupported operand type(s)"), a
245-
# numpy complex array hits the ufunc ("ufunc 'remainder' not supported"),
246-
# and pandas 1.x raises its own ("can't mod complex numbers"). The point is
247-
# that it raises at all.
245+
# Still true of the finalizer on its own. The message differs across
246+
# versions: object dtype falls back to Python's ``%`` ("unsupported operand
247+
# type(s)" on Python 3.10+, "can't mod complex numbers." on 3.9), and a
248+
# numpy complex array hits the ufunc ("ufunc 'remainder' not supported").
248249
with pytest.raises(TypeError, match="remainder|unsupported operand|mod complex"):
249250
_finalize_numeric(pd.Series([complex(1, 2), 3], dtype=object))
250251

251-
# An all-complex column is declined before reaching the finalizer, which is
252-
# the behaviour the mixed column should have as well.
252+
# The parse no longer produces a complex result for the finalizer to see.
253+
parsed = _to_numeric_or_none(pd.Series([complex(1, 2), "abc", "3"], dtype=object))
254+
assert parsed.dtype == "float64"
255+
assert parsed.isna().tolist() == [True, True, False]
256+
257+
# End to end: the mixed column is left alone, like an all-complex column.
258+
out = fd.clean(pd.DataFrame({"v": [complex(1, 2), "abc", "3"]}), verbose=False)
259+
assert out["v"].tolist() == [complex(1, 2), "abc", "3"]
253260
out = fd.clean(pd.DataFrame({"v": [complex(1, 2), complex(3, 4)]}), verbose=False)
254261
assert str(out["v"].dtype) == "complex128"
255262

256263

257-
def test_the_end_to_end_route_to_this_crash_is_order_dependent():
258-
"""Records an unexplained order dependence rather than hiding it.
259-
260-
In a fresh process ``fd.clean`` on a complex-beside-text column raises the
261-
``TypeError`` above. After some other pandas work in the same process it
262-
does not — the column stops being routed to the numeric path. Both
263-
``pd.api.types.infer_dtype`` (always ``"mixed"``) and ``_finalize_numeric``
264-
(always raises) were checked and are stable, so the divergence is upstream
265-
of the finalizer in ``clean``'s routing. The mechanism was not identified.
264+
def test_the_end_to_end_route_is_no_longer_order_dependent():
265+
"""Formerly recorded an unexplained order dependence (FD2-012).
266266
267-
This test asserts only what is stable in either state: the frame is
268-
returned unchanged, or the call raises ``TypeError`` — never a silently
269-
coerced column that has lost the complex value.
267+
It asserted only "unchanged, or ``TypeError``" because the outcome depended
268+
on what ran earlier in the process. The mechanism was pandas reading
269+
uninitialised memory for the text cells beside a complex value (see the
270+
test above). Now the outcome is the same in every state: never an
271+
exception, never a coerced column that has lost the complex value.
270272
"""
271273
frame = pd.DataFrame({"v": [complex(1, 2), "abc", "3"]})
272-
try:
273-
out = fd.clean(frame, verbose=False)
274-
except TypeError as exc:
275-
assert any(m in str(exc) for m in ("remainder", "unsupported operand", "mod complex"))
276-
return
274+
out = fd.clean(frame, verbose=False)
277275
assert out["v"].tolist() == [complex(1, 2), "abc", "3"]
278276

279277
def test_date_objects_with_a_missing_cell_still_normalize_to_datetime64():
Lines changed: 225 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,225 @@
1+
"""Exotic Python values in an otherwise numeric or textual object column.
2+
3+
FD2-012: one ``complex`` value beside text crashed ``fd.clean`` on default
4+
settings. pandas' ``to_numeric`` returns a complex result as soon as one cell
5+
is complex, and it never writes a text, bytes or bool cell into that complex
6+
buffer -- those cells come back as whatever the last same-sized buffer freed
7+
by numpy held. The garbage counted as "parsed", the column passed the numeric
8+
threshold, and the real-number finalizer's ``% 1`` raised ``TypeError``.
9+
10+
The numeric targets here are real dtypes, so a complex value is not a number
11+
this step can hold. It is treated as any other unparseable straggler: it
12+
counts against ``numeric_threshold``; when the column still converts it is
13+
quarantined (set to missing, original kept in ``report.coerced_cells``); when
14+
the column does not convert it is left exactly as it was, and the
15+
type-contamination warning names it.
16+
"""
17+
18+
import fractions
19+
20+
import numpy as np
21+
import pandas as pd
22+
import pytest
23+
24+
import freshdata as fd
25+
from freshdata.config import CleanConfig
26+
from freshdata.report import CleanReport
27+
from freshdata.steps.dtypes import (
28+
_to_numeric_or_none,
29+
refine_numeric_after_semantic,
30+
suggest_conversion,
31+
)
32+
33+
COMPLEX_VALUES = [
34+
pytest.param(complex(1, 2), id="complex"),
35+
pytest.param(np.complex128(1 + 2j), id="np.complex128"),
36+
pytest.param(np.complex64(1 + 2j), id="np.complex64"),
37+
]
38+
39+
NUMERIC_STRINGS = [str(i) for i in range(1, 21)] # 20 of 21 parse: >= 0.95
40+
41+
42+
def _clean(values, **options):
43+
out, report = fd.clean(pd.DataFrame({"v": values}), return_report=True,
44+
verbose=False, drop_duplicates=False, **options)
45+
return out["v"], report
46+
47+
48+
def _poison_numpy_buffer_cache(n: int, value: complex) -> None:
49+
"""Free an ``n``-cell complex128 buffer holding *value*.
50+
51+
numpy hands small freed buffers straight back to the next allocation of the
52+
same size, so a pandas call that reads an unwritten slot of its complex
53+
buffer sees *value*. This makes the pandas garbage deterministic instead of
54+
depending on whatever ran earlier in the process.
55+
"""
56+
buf = np.full(n, value, dtype=np.complex128)
57+
del buf
58+
59+
60+
# ── the reported crash ───────────────────────────────────────────────────────
61+
62+
63+
@pytest.mark.parametrize("value", COMPLEX_VALUES)
64+
def test_complex_beside_text_is_left_intact(value):
65+
"""The FD2-012 reproduction: the column is not numeric (one of three cells
66+
parses), so it is declined and returned exactly as given."""
67+
column, report = _clean([value, "abc", "3"])
68+
69+
assert column.dtype == object
70+
assert column.tolist() == [value, "abc", "3"]
71+
assert column.iloc[0] == complex(1, 2)
72+
assert "v" not in report.coerced_cells
73+
74+
75+
@pytest.mark.parametrize("fill", [complex(7, 7), complex(np.nan, np.nan)])
76+
def test_parse_beside_a_complex_value_does_not_read_pandas_garbage(fill):
77+
"""The cause. Whatever the freed buffer held -- a plausible number or NaN --
78+
the text cells are parsed as text: ``"abc"`` is missing, ``"3"`` is 3.0,
79+
and the complex cell is unparseable. Before the fix the result was
80+
complex128 and ``"abc"``/``"3"`` both came back as *fill*."""
81+
s = pd.Series([complex(1, 2), "abc", "3"], dtype=object)
82+
_poison_numpy_buffer_cache(len(s), fill)
83+
parsed = _to_numeric_or_none(s)
84+
85+
assert parsed is not None
86+
assert parsed.dtype == np.float64
87+
assert parsed.isna().tolist() == [True, True, False]
88+
assert parsed.iloc[2] == 3.0
89+
90+
91+
def test_result_does_not_depend_on_what_ran_earlier_in_the_process():
92+
"""The order dependence recorded in FD2-012: a NaN-filled freed buffer made
93+
the garbage fail the threshold (no crash), a 7+7j one made ``"abc"`` look
94+
parsed (crash). Both states must now give the same, intact frame."""
95+
frame = pd.DataFrame({"v": [complex(1, 2), "abc", "3"]})
96+
results = []
97+
for fill in (complex(np.nan, np.nan), complex(7, 7)):
98+
_poison_numpy_buffer_cache(len(frame), fill)
99+
results.append(fd.clean(frame, verbose=False)["v"].tolist())
100+
assert results == [[complex(1, 2), "abc", "3"]] * 2
101+
102+
103+
# ── a complex straggler in a numeric column is quarantined, not lost ─────────
104+
105+
106+
@pytest.mark.parametrize("value", COMPLEX_VALUES)
107+
def test_complex_straggler_in_a_numeric_column_is_quarantined(value):
108+
"""20 numeric strings and one complex value clear the 0.95 threshold, so the
109+
column converts. The complex value cannot live in Int64: it is set to
110+
missing, its original is in ``coerced_cells``, and a warning says so."""
111+
column, report = _clean([value, *NUMERIC_STRINGS])
112+
113+
assert str(column.dtype) == "Int64"
114+
assert column.isna().tolist() == [True] + [False] * 20
115+
assert column.iloc[1:].tolist() == list(range(1, 21))
116+
assert report.coerced_cells["v"] == {0: value}
117+
assert report.coerced_rows["v"] == (0,)
118+
assert any("could not be parsed as Int64" in w and "(1+2j) (row 0)" in w
119+
for w in report.warnings)
120+
121+
122+
def test_complex_straggler_is_handled_exactly_like_other_unparseable_values():
123+
"""Alignment with the existing straggler path: a ``Fraction`` (which pandas
124+
cannot parse either) in the same position gets the same dtype, the same
125+
missing cell and the same ``coerced_cells`` entry."""
126+
for values in ([None, *NUMERIC_STRINGS], [None, "1", "2", "3", "4"]):
127+
with_complex = _clean([complex(1, 2), *values[1:]])
128+
with_fraction = _clean([fractions.Fraction(1, 3), *values[1:]])
129+
assert str(with_complex[0].dtype) == str(with_fraction[0].dtype)
130+
assert with_complex[0].isna().tolist() == with_fraction[0].isna().tolist()
131+
assert (set(with_complex[1].coerced_cells.get("v", {}))
132+
== set(with_fraction[1].coerced_cells.get("v", {})))
133+
134+
135+
def test_complex_below_threshold_leaves_the_column_and_is_named_in_the_warning():
136+
"""Four numbers and one complex value: 80% parse, below 0.95, so nothing is
137+
converted and the contamination warning points at the complex row."""
138+
column, report = _clean([complex(1, 2), "1", "2", "3", "4"])
139+
140+
assert column.dtype == object
141+
assert column.tolist() == [complex(1, 2), "1", "2", "3", "4"]
142+
assert "v" not in report.coerced_cells
143+
assert any("looks numeric (80% of values parse)" in w and "(1+2j) (row 0)" in w
144+
and "values were NOT silently coerced" in w for w in report.warnings)
145+
146+
147+
def test_complex_beside_python_numbers_in_an_object_column_is_left_intact():
148+
"""No text at all: pandas parses this correctly (complex128), and the
149+
finalizer then crashed on ``% 1``. One of five is not real -> declined."""
150+
values = pd.Series([complex(1, 2), 1, 2, 3, 4.5], dtype=object)
151+
column, report = _clean(values)
152+
153+
assert column.dtype == object
154+
assert column.tolist() == [complex(1, 2), 1, 2, 3, 4.5]
155+
assert "v" not in report.coerced_cells
156+
157+
158+
def test_lower_threshold_quarantines_the_complex_value_beside_text():
159+
"""With ``numeric_threshold=0.3`` the FD2-012 column converts; the complex
160+
value and the word are both casualties, recorded with their originals."""
161+
column, report = _clean([complex(1, 2), "abc", "3"], numeric_threshold=0.3)
162+
163+
assert str(column.dtype) == "Int64"
164+
assert column.isna().tolist() == [True, True, False]
165+
assert column.iloc[2] == 3
166+
assert report.coerced_cells["v"] == {0: complex(1, 2), 1: "abc"}
167+
168+
169+
def test_profile_preview_matches_cleaning():
170+
"""``suggest_conversion`` is shared with ``fd.profile``; it must report the
171+
same decision instead of crashing."""
172+
config = CleanConfig()
173+
beside_text = pd.Series([complex(1, 2), "abc", "3"], dtype=object)
174+
assert suggest_conversion(beside_text, config) == ("none", None, 0)
175+
176+
target, converted, n_coerced = suggest_conversion(
177+
pd.Series([complex(1, 2), *NUMERIC_STRINGS], dtype=object), config)
178+
assert (target, str(converted.dtype), n_coerced) == ("numeric", "Int64", 1)
179+
180+
181+
def test_post_semantic_retry_declines_a_complex_casualty():
182+
"""``refine_numeric_after_semantic`` only quarantines numeric-*shaped* text.
183+
A complex value is not, so the retry leaves the column untouched."""
184+
df = pd.DataFrame({"v": [complex(1, 2), "1", "2", "3", "4", "5"]})
185+
report = CleanReport()
186+
out = refine_numeric_after_semantic(df, ["v"], CleanConfig(), report)
187+
188+
assert out["v"].tolist() == [complex(1, 2), "1", "2", "3", "4", "5"]
189+
assert "v" not in report.coerced_cells
190+
191+
192+
# ── controls: columns without a complex value convert exactly as before ──────
193+
194+
195+
@pytest.mark.parametrize(
196+
("values", "dtype", "expected"),
197+
[
198+
(["1", "2", "3"], "int64", [1, 2, 3]),
199+
(["1.5", "2", "-3"], "float64", [1.5, 2.0, -3.0]),
200+
(["1e3", "2", "-0.5"], "float64", [1000.0, 2.0, -0.5]),
201+
([str(i) for i in range(19)] + ["$1,234.56"], "float64",
202+
[float(i) for i in range(19)] + [1234.56]),
203+
(["inf", "1", "2"], "float64", [float("inf"), 1.0, 2.0]),
204+
],
205+
)
206+
def test_ordinary_numeric_text_converts_as_before(values, dtype, expected):
207+
column, report = _clean(values)
208+
assert str(column.dtype) == dtype
209+
assert column.tolist() == expected
210+
assert "v" not in report.coerced_cells
211+
212+
213+
def test_ordinary_text_straggler_is_still_quarantined_as_before():
214+
column, report = _clean(["abc", *NUMERIC_STRINGS])
215+
assert str(column.dtype) == "Int64"
216+
assert column.iloc[1:].tolist() == list(range(1, 21))
217+
assert report.coerced_cells["v"] == {0: "abc"}
218+
219+
220+
def test_all_complex_object_column_is_still_declined_before_parsing():
221+
"""``infer_dtype`` reports ``"complex"`` and the column never reaches the
222+
numeric path -- unchanged by this fix."""
223+
column, report = _clean(pd.Series([complex(1, 2), complex(3, 4)], dtype=object))
224+
assert column.tolist() == [complex(1, 2), complex(3, 4)]
225+
assert "v" not in report.coerced_cells

0 commit comments

Comments
 (0)