Skip to content

Commit 3683bb3

Browse files
fix(dtypes,strings): never abort cleaning on an unusual object cell (#453)
Three crashes on the default fd.clean path, all from code that assumed every cell in an object column is scalar text: * non-UTF-8 bytes: dtype inference cast a sample with astype("string"), and pandas 2 decodes bytes when casting to StringDtype, so a BLOB read out of a database raised UnicodeDecodeError. pandas 1.5 returned the frame with the cell untouched. Text inspection now falls back to a view that keeps str cells and ignores the rest, so both versions leave the cell alone. * pd.NA beside a list or dict: the strip and case passes counted repairs with stripped.ne(s), whose flex comparison hands object columns to NumPy, which calls bool() on pd.NA != pd.NA and raises "boolean value of NA is ambiguous". Only str cells can change, so the comparison is restricted to those positions. * booleans mixed with pd.NaT: BooleanArray accepts only None/NaN as a missing value, so [None, None, NaT, False] raised TypeError("Need to pass bool-like values") -- but only when the frame had another column, because that changed inference order. Missing cells are normalized to NaN before the boolean cast. Closes #447 Closes #448 Closes #451
1 parent e5bac8a commit 3683bb3

4 files changed

Lines changed: 142 additions & 12 deletions

File tree

‎src/freshdata/steps/dtypes.py‎

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,41 @@ def _finalize_numeric(parsed: pd.Series) -> pd.Series:
123123
return parsed.astype("float64")
124124

125125

126+
def _text_view(values: pd.Series) -> pd.Series:
127+
"""``astype("string")`` that never aborts on a cell that is not text.
128+
129+
pandas 2 *decodes* ``bytes`` when casting to ``StringDtype`` and raises
130+
``UnicodeDecodeError`` on anything that is not UTF-8 — a BLOB read straight
131+
out of a database is enough. pandas 1.5 returned such a cell untouched.
132+
Every caller here only inspects the shape of the *text* values, so on
133+
failure fall back to a view that keeps the ``str`` cells and treats anything
134+
else as missing; the original column is never modified either way.
135+
"""
136+
try:
137+
return values.astype("string")
138+
except (UnicodeDecodeError, TypeError, ValueError):
139+
return pd.Series(
140+
[v if isinstance(v, str) else pd.NA for v in values],
141+
index=values.index,
142+
dtype="string",
143+
)
144+
145+
146+
def _to_boolean(s: pd.Series) -> pd.Series:
147+
"""``astype("boolean")`` for a column whose non-missing values are bools.
148+
149+
``BooleanArray`` accepts only ``None``/``NaN`` as a missing cell, so a
150+
``pd.NaT`` — routine in a column that came out of a merge or ``read_excel``
151+
— raises ``TypeError("Need to pass bool-like values")`` even though the
152+
caller already treated it as missing via ``dropna()``. Normalize every
153+
missing cell to ``NaN`` first so the real booleans still convert.
154+
"""
155+
missing = s.isna()
156+
if missing.any():
157+
s = s.where(~missing)
158+
return s.astype("boolean")
159+
160+
126161
def _try_boolean(s: pd.Series, nonnull: pd.Series) -> pd.Series | None:
127162
"""Convert true/false-vocabulary text (or raw Python bools) to boolean."""
128163
try:
@@ -132,13 +167,13 @@ def _try_boolean(s: pd.Series, nonnull: pd.Series) -> pd.Series | None:
132167
if len(uniques) > 8: # vocabulary has at most 8 spellings
133168
return None
134169
if all(isinstance(v, bool) for v in uniques):
135-
converted = s.astype("boolean")
170+
converted = _to_boolean(s)
136171
elif all(isinstance(v, str) for v in uniques) and {
137172
v.casefold() for v in uniques
138173
} <= _BOOL_WORDS:
139174
mapping = dict.fromkeys(_TRUE_WORDS, True)
140175
mapping.update(dict.fromkeys(_FALSE_WORDS, False))
141-
converted = s.str.casefold().map(mapping).astype("boolean")
176+
converted = _to_boolean(s.str.casefold().map(mapping))
142177
else:
143178
return None
144179
if not converted.isna().any():
@@ -165,7 +200,7 @@ def _rescue_formatted(
165200
lost = s.notna() & parsed.isna()
166201
if not lost.any():
167202
return parsed
168-
strs = s[lost].astype("string")
203+
strs = _text_view(s[lost])
169204
matches = strs.str.fullmatch(formatted_re).eq(True)
170205
if matches.dtype != bool:
171206
matches = matches.fillna(False).astype(bool)
@@ -209,7 +244,7 @@ def _try_numeric(
209244
if parsed is None:
210245
# Second chance: values like "$1,234.56". Only worth attempting if the
211246
# sample actually contains separator/currency characters.
212-
has_noise = sample.astype("string").str.contains(noise_re, regex=True, na=False)
247+
has_noise = _text_view(sample).str.contains(noise_re, regex=True, na=False)
213248
if not bool(has_noise.any()):
214249
return None, 0
215250
matches = s.str.fullmatch(formatted_re).eq(True)

‎src/freshdata/steps/strings.py‎

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,24 +27,53 @@ def active_sentinels(config: CleanConfig) -> frozenset[str]:
2727
return frozenset(DEFAULT_SENTINELS | set(config.extra_sentinels))
2828

2929

30-
def _strip_series(s: pd.Series, kind: str) -> pd.Series:
30+
def _str_positions(s: pd.Series, kind: str) -> pd.Series:
31+
"""Boolean mask of the positions of *s* that hold a real ``str``.
32+
33+
These are the only cells text repair ever rewrites, so the mask drives both
34+
the repair and the count of repaired cells.
35+
"""
36+
if kind == "string":
37+
# infer_dtype("string") guarantees every non-missing value is a str.
38+
return s.notna()
39+
return s.map(lambda v: isinstance(v, str)).astype(bool)
40+
41+
42+
def _n_repaired(new: pd.Series, old: pd.Series, mask: pd.Series) -> int:
43+
"""Count the masked cells *new* actually changed.
44+
45+
Comparing the whole column instead (``new.ne(old)``) aborts the pipeline on
46+
ordinary data: the flex comparison hands object columns straight to NumPy,
47+
which calls ``bool()`` on ``pd.NA != pd.NA`` and raises "boolean value of NA
48+
is ambiguous". Any non-scalar cell (a list or dict from JSON) keeps the
49+
column object-dtype, so that path is easy to hit. Only ``str`` cells can
50+
differ here — every other cell is returned untouched by construction — so
51+
restrict the comparison to them and never look at a cell we did not repair.
52+
"""
53+
if not mask.any():
54+
return 0
55+
positions = mask.to_numpy(dtype=bool)
56+
left = new.to_numpy(dtype=object)[positions]
57+
right = old.to_numpy(dtype=object)[positions]
58+
return int((left != right).sum())
59+
60+
61+
def _strip_series(s: pd.Series, kind: str, mask: pd.Series) -> pd.Series:
3162
"""Whitespace-strip string values of *s*, preserving non-string values."""
3263
if kind == "string":
3364
return s.str.strip()
3465
# Mixed column: operate only on positions that actually hold a str.
35-
mask = s.map(lambda v: isinstance(v, str))
3666
if not mask.any():
3767
return s
3868
out = s.copy()
3969
out[mask] = s[mask].str.strip()
4070
return out
4171

4272

43-
def _case_series(s: pd.Series, kind: str, string_case: str) -> pd.Series:
73+
def _case_series(s: pd.Series, kind: str, string_case: str, mask: pd.Series) -> pd.Series:
4474
"""Case-normalize string values of *s*, preserving non-string values."""
4575
if kind == "string":
4676
return s.str.lower() if string_case == "lower" else s.str.upper()
47-
mask = s.map(lambda v: isinstance(v, str))
4877
if not mask.any():
4978
return s
5079
out = s.copy()
@@ -66,8 +95,9 @@ def normalize_text(
6695

6796
n_stripped = 0
6897
if config.strip_whitespace:
69-
stripped = _strip_series(s, kind)
70-
n_stripped = int((stripped.ne(s) & s.notna()).sum())
98+
mask = _str_positions(s, kind)
99+
stripped = _strip_series(s, kind, mask)
100+
n_stripped = _n_repaired(stripped, s, mask)
71101
if n_stripped:
72102
s = stripped
73103

@@ -83,8 +113,10 @@ def normalize_text(
83113
n_case = 0
84114
if config.string_case is not None:
85115
before = s
86-
cased = _case_series(s, kind, config.string_case)
87-
n_case = int((cased.ne(before) & before.notna()).sum())
116+
# Recomputed: the sentinel pass above may have nulled some str cells.
117+
mask = _str_positions(before, kind)
118+
cased = _case_series(before, kind, config.string_case, mask)
119+
n_case = _n_repaired(cased, before, mask)
88120
if n_case:
89121
s = cased
90122

‎tests/test_dtypes.py‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -482,3 +482,37 @@ def test_plain_datetimes_still_convert_next_to_a_time():
482482
s = clean1(["2026-01-15 09:00", "2026-02-01 10:30", "2026-03-05 11:45"],
483483
drop_duplicates=False)
484484
assert str(s.dtype).startswith("datetime64")
485+
486+
487+
# ── #447 / #451: object cells the pipeline must not choke on ────────────────────
488+
489+
490+
def test_clean_keeps_undecodable_bytes_and_cleans_the_rest():
491+
# Regression (#447): pandas 2 decodes bytes when casting to StringDtype, so
492+
# one non-UTF-8 cell (a DB BLOB) aborted the whole clean. pandas 1.5
493+
# returned the frame with the cell untouched; both do that now.
494+
df = pd.DataFrame({"a": ["$12", b"\xff"], "n": [1, 2]})
495+
out = fd.clean(df, verbose=False)
496+
assert out["a"].tolist()[1] == b"\xff"
497+
assert out["n"].tolist() == [1, 2]
498+
499+
500+
def test_clean_still_parses_ascii_bytes_columns():
501+
df = pd.DataFrame({"a": [b"ab", b"cd"], "n": [1, 2]})
502+
assert fd.clean(df, verbose=False)["a"].tolist() == [b"ab", b"cd"]
503+
504+
505+
@pytest.mark.parametrize("extra", [{"y": [0, 0, 0, 0]}, {}])
506+
def test_clean_accepts_booleans_mixed_with_nat(extra):
507+
# Regression (#451): BooleanArray rejects pd.NaT as a missing value, so
508+
# ["", NaT, False] raised TypeError("Need to pass bool-like values") — but
509+
# only when the frame had a second column, which changed inference order.
510+
df = pd.DataFrame({"x": [None, None, pd.NaT, False], **extra})
511+
out = fd.clean(df, verbose=False, drop_empty_rows=False)
512+
assert out["x"].isna().tolist()[:3] == [True, True, True]
513+
assert bool(out["x"].tolist()[3]) is False
514+
515+
516+
def test_boolean_columns_without_missing_values_still_convert():
517+
df = pd.DataFrame({"x": [True, False, True, False]})
518+
assert str(fd.clean(df, verbose=False)["x"].dtype) in {"bool", "boolean"}

‎tests/test_strings.py‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,3 +149,32 @@ def test_categorical_values_match_object_column():
149149
out_obj = fd.clean(cat.astype({"c": object}), verbose=False)
150150
assert isinstance(out_cat["c"].dtype, pd.CategoricalDtype)
151151
assert _plain(out_cat["c"].astype(object)) == _plain(out_obj["c"])
152+
153+
154+
def test_clean_handles_missing_values_next_to_a_container_cell():
155+
# Regression (#448): the strip pass counted repairs with stripped.ne(s),
156+
# whose flex comparison hands object columns to NumPy, which calls bool()
157+
# on pd.NA != pd.NA and raises "boolean value of NA is ambiguous". A list
158+
# cell keeps the column object-dtype, which is how JSON data arrives.
159+
df = pd.DataFrame({"a": [pd.NA, []], "keep": [1, 2]})
160+
out = fd.clean(df, verbose=False)
161+
assert out["keep"].tolist() == [1, 2]
162+
assert [] in out["a"].tolist()
163+
164+
165+
@pytest.mark.parametrize("cell", [[], {"k": 1}, {1, 2}, (1,)])
166+
def test_text_repair_leaves_container_cells_untouched(cell):
167+
df = pd.DataFrame({"a": [cell, " padded ", pd.NA], "n": [1, 2, 3]})
168+
out = fd.clean(df, verbose=False, drop_empty_rows=False)
169+
values = out["a"].tolist()
170+
assert values[0] == cell # containers are never rewritten
171+
assert values[1] == "padded" # ordinary text is still stripped
172+
173+
174+
def test_repair_counts_ignore_untouched_container_cells():
175+
df = pd.DataFrame({"a": [[1], " x ", " y "], "n": [1, 2, 3]})
176+
_, report = fd.clean(df, verbose=False, return_report=True, drop_empty_rows=False)
177+
stripped = [
178+
a for a in report.actions if a.step == "strip_whitespace" and a.column == "a"
179+
]
180+
assert stripped and all(a.count == 2 for a in stripped) # the list cell is not counted

0 commit comments

Comments
 (0)