From 61e7f3db1691322100ffb25db78a559a95657b6e Mon Sep 17 00:00:00 2001 From: Kevin Costner <120246174+kevincostner17@users.noreply.github.com> Date: Mon, 28 Sep 2026 23:26:20 +0530 Subject: [PATCH] fix(dtypes): the report no longer calls a datetime parse "converted to object" A column of datetime strings carrying different UTC offsets -- the ordinary result of a DST changeover or of merging exports from two regions -- is parsed cell by cell into timestamps that each keep their own offset. No single datetime64 dtype can hold several offsets, so pandas returns them in an object column. The parse is faithful and lossless, but fix_dtypes built its description from the dtype alone and recorded "converted to object", risk low -- which reads as though nothing was converted. The description now says what happened ("parsed to timestamps; kept as object because the values carry different UTC offsets, so no single datetime64 dtype can hold them", or "... mix timezone-aware and timezone-naive timestamps"), and the coercion warning says "could not be parsed as timestamps" instead of "as object". Report text only: cell values, dtypes, risk and confidence are unchanged (checked byte-for-byte against main over 12 cases), as is every other column's description. The "after semantic repair" sibling description cannot receive such a column (refine_numeric_after_semantic only produces real numeric dtypes); a test pins that. tests/test_mixed_offset_audit_record.py: 20 tests. With the updated threshold test, 12 fail without this change on both py3.12/pandas 2.3.3 and py3.9/pandas 1.5.3. --- CHANGELOG.md | 28 +++ src/freshdata/steps/dtypes.py | 58 ++++- tests/test_dtypes_threshold_mutants.py | 28 +-- tests/test_mixed_offset_audit_record.py | 281 ++++++++++++++++++++++++ 4 files changed, 378 insertions(+), 17 deletions(-) create mode 100644 tests/test_mixed_offset_audit_record.py diff --git a/CHANGELOG.md b/CHANGELOG.md index fbc30c5..988541f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -113,6 +113,34 @@ adheres to [Semantic Versioning](https://semver.org/). entirely of complex values is still declined before parsing and keeps its dtype. +- The report no longer calls a datetime parse "converted to object". A column + of datetime strings carrying different UTC offsets — `"2021-01-05 + 00:00:00+01:00"`, `"2021-01-06 00:00:00+02:00"`, the ordinary result of a + daylight-saving changeover or of merging exports from two regions — is parsed + cell by cell into timestamps that each keep their own offset. No single + `datetime64` dtype can hold several offsets, so pandas returns them in an + `object` column. That parse is faithful and lossless, but `fix_dtypes` built + its action description from the resulting dtype alone and recorded it as + `converted to object`, risk low, which reads as though nothing was + converted. A column mixing values with and without an offset took the same + path and got the same record. The description now says what happened: + `parsed to timestamps; kept as object because the values carry different + UTC offsets, so no single datetime64 dtype can hold them` (or `... because + the values mix timezone-aware and timezone-naive timestamps, ...`), followed + by the existing ` (N unparseable value(s) set to missing)` suffix when cells + were coerced. The coercion warning for such a column likewise says the + values "could not be parsed as timestamps" instead of "as object". Affects + every supported pandas version (pandas 1.x delivers `datetime.datetime` + cells, pandas 2 `Timestamp` cells; the record is the same). + + **Compatibility impact:** report text only. Cell values, dtypes, risk + levels and confidence are unchanged, and so is every other column's + `converted to ` description, including single-offset + (`datetime64[ns, UTC+01:00]`) and naive (`datetime64[ns]`) datetime + columns. Code that matched the literal string `converted to object` for + such a column needs to match the new description. `fd.profile` still + previews such a column as `would convert to object`. + ### Documentation diff --git a/src/freshdata/steps/dtypes.py b/src/freshdata/steps/dtypes.py index a581263..9d57032 100644 --- a/src/freshdata/steps/dtypes.py +++ b/src/freshdata/steps/dtypes.py @@ -592,8 +592,56 @@ def _warn_type_contamination(col: str, s: pd.Series, config: CleanConfig, COERCED_CELLS_CAP = 1_000 +def _is_object_datetime(target: str, converted: pd.Series) -> bool: + """A datetime parse that pandas could only deliver as an object column. + + Values carrying different UTC offsets, or mixing timezone-aware with + timezone-naive values, have no common ``datetime64`` dtype, so pandas + returns one timestamp object per cell in an ``object`` column. The cells + were parsed; only the dtype is not a datetime64 one. + """ + return target == "datetime" and not is_datetime64_any_dtype(converted.dtype) + + +def _object_datetime_reason(converted: pd.Series) -> str: + """Say why the parsed timestamps in *converted* share no datetime64 dtype.""" + aware = naive = False + offsets = set() + for value in converted.dropna(): + utcoffset = getattr(value, "utcoffset", None) + if utcoffset is None: + continue + offset = utcoffset() + if offset is None: + naive = True + else: + aware = True + offsets.add(offset) + if aware and naive: + why = "the values mix timezone-aware and timezone-naive timestamps" + elif len(offsets) > 1: + why = "the values carry different UTC offsets" + else: + return "no single datetime64 dtype can hold them" + return f"{why}, so no single datetime64 dtype can hold them" + + +def _describe_conversion(target: str, converted: pd.Series) -> str: + """The audit description of what :func:`fix_dtypes` did to a column. + + ``"converted to "`` except when a datetime parse ended in an object + column: saying "converted to object" there reads as though nothing was + converted, when every cell was in fact parsed to a timestamp. + """ + if _is_object_datetime(target, converted): + return (f"parsed to timestamps; kept as {converted.dtype} because " + f"{_object_datetime_reason(converted)}") + return f"converted to {converted.dtype}" + + def _record_coerced(col: str, before: pd.Series, converted: pd.Series, - report: CleanReport, config: CleanConfig) -> None: + report: CleanReport, config: CleanConfig, + target: str = "") -> None: """Preserve the original value of every cell the conversion nulled. For declared ``sensitive_columns`` the row keys survive but every value is @@ -615,9 +663,11 @@ def _record_coerced(col: str, before: pd.Series, converted: pd.Series, for i, v in list(originals.head(3).items())) truncated = "" if len(originals) <= COERCED_CELLS_CAP else ( f"; first {COERCED_CELLS_CAP} recorded") + parsed_as = ("timestamps" if _is_object_datetime(target, converted) + else str(converted.dtype)) report.add_warning( f"column '{col}': {len(originals)} value(s) could not be parsed as " - f"{converted.dtype} and were set to missing — e.g. {examples}. " + f"{parsed_as} and were set to missing — e.g. {examples}. " f"Originals are preserved in report.coerced_cells{truncated}; these " "cells stay missing (never auto-imputed) so they can be reviewed." ) @@ -726,10 +776,10 @@ def fix_dtypes(df: pd.DataFrame, config: CleanConfig, report: CleanReport) -> pd if converted is None: _warn_type_contamination(str(col), s, config, report) continue - description = f"converted to {converted.dtype}" + description = _describe_conversion(target, converted) if n_coerced: description += f" ({n_coerced} unparseable value(s) set to missing)" - _record_coerced(str(col), s, converted, report, config) + _record_coerced(str(col), s, converted, report, config, target) report.add("fix_dtypes", description, column=str(col), count=int(converted.notna().sum()) + n_coerced) df[col] = converted diff --git a/tests/test_dtypes_threshold_mutants.py b/tests/test_dtypes_threshold_mutants.py index 95ed909..4722f31 100644 --- a/tests/test_dtypes_threshold_mutants.py +++ b/tests/test_dtypes_threshold_mutants.py @@ -482,20 +482,21 @@ def test_the_post_semantic_retry_tolerates_exactly_the_casualty_budget(): # --------------------------------------------------------------------------- -def test_mixed_utc_offsets_are_reported_as_a_conversion_to_object(): - """Pins current behaviour found while covering bool#16 -- a defect. +def test_mixed_utc_offsets_parse_to_an_object_column_of_timestamps(): + """Pins current behaviour found while covering bool#16. Values carrying *different* UTC offsets cannot share a datetime64 column, - so pandas returns an object-dtype Series of Timestamps. ``_try_datetime`` - measures that object Series against the threshold, passes it, and the - column is announced as a dtype conversion: the report says "converted to - object" and the frame keeps an object column. A conversion that ends in - ``object`` is not a conversion, and because the result is not - datetime64 the ambiguity note in ``_record_coerced`` can never fire for - such a column either. - - Nothing here argues the behaviour is right; it is pinned so that fixing - it has to change this test deliberately. + so pandas returns an object-dtype Series of per-cell timestamps, each + keeping its own offset. ``_try_datetime`` measures that object Series + against the threshold, passes it, and the column keeps the parsed + timestamps in an object column. + + The audit record used to say "converted to object", which reads as though + nothing was converted (FD2-015); it now says what happened. The exact + wording is pinned in ``tests/test_mixed_offset_audit_record.py``. Whether + such a column should be converted at all (``utc=True``, or declined) is + not decided here; the dtype is pinned so that changing it has to change + this test deliberately. """ values = [ "2021-01-05 00:00:00+01:00", @@ -510,4 +511,5 @@ def test_mixed_utc_offsets_are_reported_as_a_conversion_to_object(): report = CleanReport() frame = fix_dtypes(pd.DataFrame({"when": values}), CleanConfig(), report) assert str(frame["when"].dtype) == "object" - assert [a.description for a in report.actions] == ["converted to object"] + [description] = [a.description for a in report.actions] + assert description.startswith("parsed to timestamps; kept as object") diff --git a/tests/test_mixed_offset_audit_record.py b/tests/test_mixed_offset_audit_record.py new file mode 100644 index 0000000..6cba7c0 --- /dev/null +++ b/tests/test_mixed_offset_audit_record.py @@ -0,0 +1,281 @@ +"""The audit record of a datetime parse that pandas can only keep as ``object``. + +Datetime strings carrying *different* UTC offsets (or mixing offset-bearing +and offset-free values) have no common ``datetime64`` dtype. pandas parses +every cell to its own timestamp, each keeping its own offset, and returns them +in an ``object`` column. That parse is faithful and lossless, and ``fix_dtypes`` +delivers it -- but it used to record the action as ``"converted to object"``, +which reads as though nothing was converted (FD2-015). + +These tests pin the record, not the data: the description now says the +values were parsed to timestamps and why they stay ``object``, while the cell +values, the dtype, the risk and the confidence stay exactly what they were. +Every other column keeps its ``"converted to "`` description. +""" +from __future__ import annotations + +import datetime as dt + +import pandas as pd +import pytest + +import freshdata as fd +from freshdata.config import CleanConfig +from freshdata.report import CleanReport +from freshdata.steps.dtypes import ( + fix_dtypes, + refine_numeric_after_semantic, + suggest_conversion, +) + +MIXED_OFFSETS = [ + "2021-01-05 00:00:00+01:00", + "2021-01-06 00:00:00+02:00", + "2021-01-07 00:00:00+03:00", + "2021-01-08 00:00:00+04:00", +] + +#: Enough distinct values that one junk cell stays under the 95% threshold. +MIXED_OFFSETS_WITH_JUNK = [ + f"2021-01-{d:02d} 00:00:00+0{d % 5}:00" for d in range(1, 26) +] + ["garbage"] + +NAIVE_AND_AWARE = [ + "2021-01-05 00:00:00", + "2021-01-06 00:00:00+01:00", + "2021-01-07 00:00:00", + "2021-01-08 00:00:00", +] + +DIFFERENT_OFFSETS = ( + "parsed to timestamps; kept as object because the values carry different " + "UTC offsets, so no single datetime64 dtype can hold them" +) +NAIVE_MIXED_WITH_AWARE = ( + "parsed to timestamps; kept as object because the values mix " + "timezone-aware and timezone-naive timestamps, so no single datetime64 " + "dtype can hold them" +) +ONE_CASUALTY = " (1 unparseable value(s) set to missing)" + + +def _fix(values): + """``fix_dtypes`` on one column named ``t``: ``(frame, report)``.""" + report = CleanReport() + frame = fix_dtypes(pd.DataFrame({"t": values}), CleanConfig(), report) + return frame, report + + +def _only_action(report: CleanReport): + [action] = [a for a in report.actions if a.step == "fix_dtypes"] + return action + + +# --------------------------------------------------------------------------- +# The record says what happened +# --------------------------------------------------------------------------- + + +def test_mixed_offsets_are_recorded_as_parsed_timestamps_kept_as_object(): + frame, report = _fix(MIXED_OFFSETS) + assert str(frame["t"].dtype) == "object" + action = _only_action(report) + assert action.description == DIFFERENT_OFFSETS + assert action.column == "t" + assert action.count == 4 + + +@pytest.mark.parametrize( + "values", + [ + pytest.param( + ["2021-01-05T10:00:00+01:00", "2021-03-05T10:00:00+01:00", + "2021-07-05T10:00:00+02:00", "2021-08-05T10:00:00+02:00"], + id="dst-changeover", + ), + pytest.param( + ["2021-01-05T10:00:00Z", "2021-01-06T10:00:00Z", + "2021-01-07T10:00:00+01:00", "2021-01-08T10:00:00Z"], + id="z-and-offset", + ), + pytest.param( + ["2021-01-05 00:00:00.123456+01:00", "2021-01-06 00:00:00.5+02:00", + "2021-01-07 00:00:00+03:00", "2021-01-08 00:00:00+04:00"], + id="fractional-seconds", + ), + ], +) +def test_every_different_offset_shape_gets_the_same_record(values): + frame, report = _fix(values) + assert str(frame["t"].dtype) == "object" + assert _only_action(report).description == DIFFERENT_OFFSETS + + +def test_mixed_offsets_in_a_string_dtype_column_get_the_same_record(): + frame, report = _fix(pd.Series(MIXED_OFFSETS, dtype="string")) + assert str(frame["t"].dtype) == "object" + assert _only_action(report).description == DIFFERENT_OFFSETS + + +def test_naive_mixed_with_aware_values_names_that_reason(): + """Same mechanism, different cause: some values have no offset at all.""" + frame, report = _fix(NAIVE_AND_AWARE) + assert str(frame["t"].dtype) == "object" + assert _only_action(report).description == NAIVE_MIXED_WITH_AWARE + + +def test_the_public_clean_report_carries_the_record(): + out, report = fd.clean(pd.DataFrame({"t": MIXED_OFFSETS}), + return_report=True, verbose=False) + assert str(out["t"].dtype) == "object" + assert [a.description for a in report.actions if a.step == "fix_dtypes"] == [ + DIFFERENT_OFFSETS + ] + + +def test_risk_and_confidence_are_not_changed_by_the_new_record(): + action = _only_action(_fix(MIXED_OFFSETS)[1]) + assert action.description == DIFFERENT_OFFSETS + assert (action.risk, action.confidence) == ("low", 1.0) + + +def test_description_helper_falls_back_to_a_generic_reason(): + """No probed input reaches this branch; it keeps the record truthful anyway.""" + from freshdata.steps.dtypes import _describe_conversion # noqa: PLC0415 + + dates = pd.Series([dt.date(2021, 1, 5), dt.date(2021, 1, 6)], dtype=object) + assert _describe_conversion("datetime", dates) == ( + "parsed to timestamps; kept as object because no single datetime64 " + "dtype can hold them" + ) + # A non-datetime target keeps the plain dtype description. + assert _describe_conversion("numeric", dates) == "converted to object" + + +# --------------------------------------------------------------------------- +# (c) casualties keep their suffix and their warning +# --------------------------------------------------------------------------- + + +def test_unparseable_suffix_is_kept_after_the_new_description(): + frame, report = _fix(MIXED_OFFSETS_WITH_JUNK) + assert str(frame["t"].dtype) == "object" + assert _only_action(report).description == DIFFERENT_OFFSETS + ONE_CASUALTY + assert report.coerced_cells == {"t": {25: "garbage"}} + [warning] = report.warnings + assert warning.startswith( + "column 't': 1 value(s) could not be parsed as timestamps and were set " + "to missing — e.g. 'garbage' (row 25)." + ) + + +def test_naive_and_aware_casualty_keeps_its_suffix(): + values = [ + f"2021-02-{d:02d} 00:00:00" + ("+01:00" if d % 2 else "") for d in range(1, 26) + ] + ["garbage"] + _, report = _fix(values) + assert _only_action(report).description == NAIVE_MIXED_WITH_AWARE + ONE_CASUALTY + + +# --------------------------------------------------------------------------- +# (a) controls: every datetime64 conversion keeps its description exactly +# --------------------------------------------------------------------------- + + +def test_single_offset_column_keeps_its_description(): + values = [v[:-6] + "+01:00" for v in MIXED_OFFSETS] + frame, report = _fix(values) + dtype = frame["t"].dtype + assert isinstance(dtype, pd.DatetimeTZDtype) + # ``UTC+01:00`` on pandas 2, ``pytz.FixedOffset(60)`` on pandas 1.x. + assert _only_action(report).description == f"converted to {dtype}" + + +def test_naive_column_keeps_its_description(): + frame, report = _fix([v[:10] for v in MIXED_OFFSETS]) + assert str(frame["t"].dtype) == "datetime64[ns]" + assert _only_action(report).description == "converted to datetime64[ns]" + + +def test_naive_column_with_a_casualty_keeps_its_description_and_warning(): + values = [f"2021-01-{d:02d}" for d in range(1, 26)] + ["garbage"] + frame, report = _fix(values) + assert str(frame["t"].dtype) == "datetime64[ns]" + assert _only_action(report).description == ( + "converted to datetime64[ns]" + ONE_CASUALTY + ) + [warning] = report.warnings + assert "could not be parsed as datetime64[ns] and were set to missing" in warning + + +def test_single_offset_column_with_a_casualty_keeps_its_description(): + values = [f"2021-01-{d:02d} 00:00:00+01:00" for d in range(1, 26)] + ["garbage"] + frame, report = _fix(values) + dtype = frame["t"].dtype + assert isinstance(dtype, pd.DatetimeTZDtype) + assert _only_action(report).description == f"converted to {dtype}" + ONE_CASUALTY + + +@pytest.mark.parametrize( + ("values", "expected"), + [ + (["1", "2", "3", "4.5"], "converted to float64"), + (["yes", "no", "yes", "no"], "converted to bool"), + ], +) +def test_non_datetime_conversions_keep_their_description(values, expected): + assert _only_action(_fix(values)[1]).description == expected + + +# --------------------------------------------------------------------------- +# (b) the data is exactly what it was before the record changed +# --------------------------------------------------------------------------- + + +def test_mixed_offset_cells_are_unchanged(): + """Pinned against main's output: one timestamp per cell, own offset kept.""" + frame, _ = _fix(MIXED_OFFSETS) + column = frame["t"] + assert str(column.dtype) == "object" + assert [cell.isoformat() for cell in column] == [ + "2021-01-05T00:00:00+01:00", + "2021-01-06T00:00:00+02:00", + "2021-01-07T00:00:00+03:00", + "2021-01-08T00:00:00+04:00", + ] + assert all(isinstance(cell, dt.datetime) for cell in column) + # Exactly the series ``suggest_conversion`` produced: the record is the + # only thing fix_dtypes adds. + target, converted, _ = suggest_conversion(pd.Series(MIXED_OFFSETS), CleanConfig()) + assert target == "datetime" + assert column.equals(converted) + assert [type(c) for c in column] == [type(c) for c in converted] + + +def test_mixed_offset_cells_with_a_casualty_are_unchanged(): + frame, _ = _fix(MIXED_OFFSETS_WITH_JUNK) + column = frame["t"] + assert str(column.dtype) == "object" + assert [cell.isoformat() for cell in column.iloc[:-1]] == [ + f"2021-01-{d:02d}T00:00:00+0{d % 5}:00" for d in range(1, 26) + ] + assert column.iloc[-1] is pd.NaT + + +# --------------------------------------------------------------------------- +# The post-semantic numeric retry cannot produce this situation +# --------------------------------------------------------------------------- + + +def test_semantic_numeric_retry_leaves_parsed_timestamps_alone(): + """``refine_numeric_after_semantic`` only ever yields int64/Int64/float64. + + Even handed the object column of timestamps directly, nothing parses as a + number, so it declines and records nothing -- its own "converted to ... + after semantic repair" description never sees an object result. + """ + frame, _ = _fix(MIXED_OFFSETS) + report = CleanReport() + out = refine_numeric_after_semantic(frame.copy(), ["t"], CleanConfig(), report) + assert report.actions == [] + assert out["t"].equals(frame["t"])