From ffc5f2945bd9a6ce43ada5828e62682de7c9d886 Mon Sep 17 00:00:00 2001 From: Kevin Costner <120246174+kevincostner17@users.noreply.github.com> Date: Wed, 16 Sep 2026 22:57:26 +0530 Subject: [PATCH] fix(dtypes,strings): never abort cleaning on an unusual object cell 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 --- src/freshdata/steps/dtypes.py | 43 +++++++++++++++++++++++++++--- src/freshdata/steps/strings.py | 48 ++++++++++++++++++++++++++++------ tests/test_dtypes.py | 34 ++++++++++++++++++++++++ tests/test_strings.py | 29 ++++++++++++++++++++ 4 files changed, 142 insertions(+), 12 deletions(-) diff --git a/src/freshdata/steps/dtypes.py b/src/freshdata/steps/dtypes.py index c2121858..263d3ec4 100644 --- a/src/freshdata/steps/dtypes.py +++ b/src/freshdata/steps/dtypes.py @@ -123,6 +123,41 @@ def _finalize_numeric(parsed: pd.Series) -> pd.Series: return parsed.astype("float64") +def _text_view(values: pd.Series) -> pd.Series: + """``astype("string")`` that never aborts on a cell that is not text. + + pandas 2 *decodes* ``bytes`` when casting to ``StringDtype`` and raises + ``UnicodeDecodeError`` on anything that is not UTF-8 — a BLOB read straight + out of a database is enough. pandas 1.5 returned such a cell untouched. + Every caller here only inspects the shape of the *text* values, so on + failure fall back to a view that keeps the ``str`` cells and treats anything + else as missing; the original column is never modified either way. + """ + try: + return values.astype("string") + except (UnicodeDecodeError, TypeError, ValueError): + return pd.Series( + [v if isinstance(v, str) else pd.NA for v in values], + index=values.index, + dtype="string", + ) + + +def _to_boolean(s: pd.Series) -> pd.Series: + """``astype("boolean")`` for a column whose non-missing values are bools. + + ``BooleanArray`` accepts only ``None``/``NaN`` as a missing cell, so a + ``pd.NaT`` — routine in a column that came out of a merge or ``read_excel`` + — raises ``TypeError("Need to pass bool-like values")`` even though the + caller already treated it as missing via ``dropna()``. Normalize every + missing cell to ``NaN`` first so the real booleans still convert. + """ + missing = s.isna() + if missing.any(): + s = s.where(~missing) + return s.astype("boolean") + + def _try_boolean(s: pd.Series, nonnull: pd.Series) -> pd.Series | None: """Convert true/false-vocabulary text (or raw Python bools) to boolean.""" try: @@ -132,13 +167,13 @@ def _try_boolean(s: pd.Series, nonnull: pd.Series) -> pd.Series | None: if len(uniques) > 8: # vocabulary has at most 8 spellings return None if all(isinstance(v, bool) for v in uniques): - converted = s.astype("boolean") + converted = _to_boolean(s) elif all(isinstance(v, str) for v in uniques) and { v.casefold() for v in uniques } <= _BOOL_WORDS: mapping = dict.fromkeys(_TRUE_WORDS, True) mapping.update(dict.fromkeys(_FALSE_WORDS, False)) - converted = s.str.casefold().map(mapping).astype("boolean") + converted = _to_boolean(s.str.casefold().map(mapping)) else: return None if not converted.isna().any(): @@ -165,7 +200,7 @@ def _rescue_formatted( lost = s.notna() & parsed.isna() if not lost.any(): return parsed - strs = s[lost].astype("string") + strs = _text_view(s[lost]) matches = strs.str.fullmatch(formatted_re).eq(True) if matches.dtype != bool: matches = matches.fillna(False).astype(bool) @@ -209,7 +244,7 @@ def _try_numeric( if parsed is None: # Second chance: values like "$1,234.56". Only worth attempting if the # sample actually contains separator/currency characters. - has_noise = sample.astype("string").str.contains(noise_re, regex=True, na=False) + has_noise = _text_view(sample).str.contains(noise_re, regex=True, na=False) if not bool(has_noise.any()): return None, 0 matches = s.str.fullmatch(formatted_re).eq(True) diff --git a/src/freshdata/steps/strings.py b/src/freshdata/steps/strings.py index 07444172..d89c2503 100644 --- a/src/freshdata/steps/strings.py +++ b/src/freshdata/steps/strings.py @@ -27,12 +27,42 @@ def active_sentinels(config: CleanConfig) -> frozenset[str]: return frozenset(DEFAULT_SENTINELS | set(config.extra_sentinels)) -def _strip_series(s: pd.Series, kind: str) -> pd.Series: +def _str_positions(s: pd.Series, kind: str) -> pd.Series: + """Boolean mask of the positions of *s* that hold a real ``str``. + + These are the only cells text repair ever rewrites, so the mask drives both + the repair and the count of repaired cells. + """ + if kind == "string": + # infer_dtype("string") guarantees every non-missing value is a str. + return s.notna() + return s.map(lambda v: isinstance(v, str)).astype(bool) + + +def _n_repaired(new: pd.Series, old: pd.Series, mask: pd.Series) -> int: + """Count the masked cells *new* actually changed. + + Comparing the whole column instead (``new.ne(old)``) aborts the pipeline on + ordinary data: the flex comparison hands object columns straight to NumPy, + which calls ``bool()`` on ``pd.NA != pd.NA`` and raises "boolean value of NA + is ambiguous". Any non-scalar cell (a list or dict from JSON) keeps the + column object-dtype, so that path is easy to hit. Only ``str`` cells can + differ here — every other cell is returned untouched by construction — so + restrict the comparison to them and never look at a cell we did not repair. + """ + if not mask.any(): + return 0 + positions = mask.to_numpy(dtype=bool) + left = new.to_numpy(dtype=object)[positions] + right = old.to_numpy(dtype=object)[positions] + return int((left != right).sum()) + + +def _strip_series(s: pd.Series, kind: str, mask: pd.Series) -> pd.Series: """Whitespace-strip string values of *s*, preserving non-string values.""" if kind == "string": return s.str.strip() # Mixed column: operate only on positions that actually hold a str. - mask = s.map(lambda v: isinstance(v, str)) if not mask.any(): return s out = s.copy() @@ -40,11 +70,10 @@ def _strip_series(s: pd.Series, kind: str) -> pd.Series: return out -def _case_series(s: pd.Series, kind: str, string_case: str) -> pd.Series: +def _case_series(s: pd.Series, kind: str, string_case: str, mask: pd.Series) -> pd.Series: """Case-normalize string values of *s*, preserving non-string values.""" if kind == "string": return s.str.lower() if string_case == "lower" else s.str.upper() - mask = s.map(lambda v: isinstance(v, str)) if not mask.any(): return s out = s.copy() @@ -66,8 +95,9 @@ def normalize_text( n_stripped = 0 if config.strip_whitespace: - stripped = _strip_series(s, kind) - n_stripped = int((stripped.ne(s) & s.notna()).sum()) + mask = _str_positions(s, kind) + stripped = _strip_series(s, kind, mask) + n_stripped = _n_repaired(stripped, s, mask) if n_stripped: s = stripped @@ -83,8 +113,10 @@ def normalize_text( n_case = 0 if config.string_case is not None: before = s - cased = _case_series(s, kind, config.string_case) - n_case = int((cased.ne(before) & before.notna()).sum()) + # Recomputed: the sentinel pass above may have nulled some str cells. + mask = _str_positions(before, kind) + cased = _case_series(before, kind, config.string_case, mask) + n_case = _n_repaired(cased, before, mask) if n_case: s = cased diff --git a/tests/test_dtypes.py b/tests/test_dtypes.py index fd58db85..8292f960 100644 --- a/tests/test_dtypes.py +++ b/tests/test_dtypes.py @@ -482,3 +482,37 @@ def test_plain_datetimes_still_convert_next_to_a_time(): s = clean1(["2026-01-15 09:00", "2026-02-01 10:30", "2026-03-05 11:45"], drop_duplicates=False) assert str(s.dtype).startswith("datetime64") + + +# ── #447 / #451: object cells the pipeline must not choke on ──────────────────── + + +def test_clean_keeps_undecodable_bytes_and_cleans_the_rest(): + # Regression (#447): pandas 2 decodes bytes when casting to StringDtype, so + # one non-UTF-8 cell (a DB BLOB) aborted the whole clean. pandas 1.5 + # returned the frame with the cell untouched; both do that now. + df = pd.DataFrame({"a": ["$12", b"\xff"], "n": [1, 2]}) + out = fd.clean(df, verbose=False) + assert out["a"].tolist()[1] == b"\xff" + assert out["n"].tolist() == [1, 2] + + +def test_clean_still_parses_ascii_bytes_columns(): + df = pd.DataFrame({"a": [b"ab", b"cd"], "n": [1, 2]}) + assert fd.clean(df, verbose=False)["a"].tolist() == [b"ab", b"cd"] + + +@pytest.mark.parametrize("extra", [{"y": [0, 0, 0, 0]}, {}]) +def test_clean_accepts_booleans_mixed_with_nat(extra): + # Regression (#451): BooleanArray rejects pd.NaT as a missing value, so + # ["", NaT, False] raised TypeError("Need to pass bool-like values") — but + # only when the frame had a second column, which changed inference order. + df = pd.DataFrame({"x": [None, None, pd.NaT, False], **extra}) + out = fd.clean(df, verbose=False, drop_empty_rows=False) + assert out["x"].isna().tolist()[:3] == [True, True, True] + assert bool(out["x"].tolist()[3]) is False + + +def test_boolean_columns_without_missing_values_still_convert(): + df = pd.DataFrame({"x": [True, False, True, False]}) + assert str(fd.clean(df, verbose=False)["x"].dtype) in {"bool", "boolean"} diff --git a/tests/test_strings.py b/tests/test_strings.py index f6d9c7f9..4fde89b7 100644 --- a/tests/test_strings.py +++ b/tests/test_strings.py @@ -149,3 +149,32 @@ def test_categorical_values_match_object_column(): out_obj = fd.clean(cat.astype({"c": object}), verbose=False) assert isinstance(out_cat["c"].dtype, pd.CategoricalDtype) assert _plain(out_cat["c"].astype(object)) == _plain(out_obj["c"]) + + +def test_clean_handles_missing_values_next_to_a_container_cell(): + # Regression (#448): the strip pass 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". A list + # cell keeps the column object-dtype, which is how JSON data arrives. + df = pd.DataFrame({"a": [pd.NA, []], "keep": [1, 2]}) + out = fd.clean(df, verbose=False) + assert out["keep"].tolist() == [1, 2] + assert [] in out["a"].tolist() + + +@pytest.mark.parametrize("cell", [[], {"k": 1}, {1, 2}, (1,)]) +def test_text_repair_leaves_container_cells_untouched(cell): + df = pd.DataFrame({"a": [cell, " padded ", pd.NA], "n": [1, 2, 3]}) + out = fd.clean(df, verbose=False, drop_empty_rows=False) + values = out["a"].tolist() + assert values[0] == cell # containers are never rewritten + assert values[1] == "padded" # ordinary text is still stripped + + +def test_repair_counts_ignore_untouched_container_cells(): + df = pd.DataFrame({"a": [[1], " x ", " y "], "n": [1, 2, 3]}) + _, report = fd.clean(df, verbose=False, return_report=True, drop_empty_rows=False) + stripped = [ + a for a in report.actions if a.step == "strip_whitespace" and a.column == "a" + ] + assert stripped and all(a.count == 2 for a in stripped) # the list cell is not counted