diff --git a/CHANGELOG.md b/CHANGELOG.md index 32e6536..acc87e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,34 @@ adheres to [Semantic Versioning](https://semver.org/). Frames with repeated labels that previously raised now clean, and produce the same result as the identical frame with a unique index. +- A time of day no longer defeats the `dayfirst="auto"` quarantine. Under the + default `dayfirst="auto"`, day/month-ambiguous dates are documented to be + quarantined for review, never read in an inferred order — and a bare + `"05/01/2021"` was. But `"05/01/2021 00:00"` was silently read as **1 May** + and reported as a plain `converted to datetime64[ns]`, with no quarantine, + no ambiguity note and no warning. On a day-first export every date landed in + the wrong month. The detector matched only a bare `DD/MM/YYYY`: its pattern + was anchored at `$` straight after the year, so any time suffix stopped the + match and the value fell through to the parser, where `"auto"` becomes + month-first. The detector now *searches* for the numeric date token anywhere + in the value instead of matching the whole string, since no time, weekday, + AM/PM marker or UTC offset around it says which part is the month. Searching + makes it fail closed: a first fix that listed allowed time suffixes still let + `"05/01/2021, 10:00"`, `"Mon 05/01/2021"`, `"05/01/2021."`, `"05/01/2021 10h"` + and `"05/01/2021 at 10:00"` through, because any such list is incomplete. + Affects every supported pandas version. The finance domain validator already + treated timed values as ambiguous; the core dtype step now agrees with it. + + **Compatibility impact:** output changes for columns of numeric dates *with + anything around them* (a time, weekday, offset, trailing punctuation) where + both day and month are <= 12, cleaned with the default + `dayfirst="auto"`. Such values are now quarantined — set to missing, with + the original kept in `report.coerced_cells` — exactly as the same dates + without a time always were, and a column made entirely of them stays text. + Previously they were converted, month-first. To convert them, state the + order: `dayfirst=True` or `dayfirst=False`. Unambiguous values + (`"13/01/2021 10:00"`) and explicit `dayfirst` settings are unchanged. + - `dayfirst=True` no longer reinterprets unambiguous ISO-8601 dates. `fd.clean(df, dayfirst=True)` on `["2021-01-05", "2021-02-11"]` silently returned `2021-05-01`, `2021-11-02` — month and day swapped, with no warning diff --git a/src/freshdata/steps/dtypes.py b/src/freshdata/steps/dtypes.py index ac39950..f14a8c6 100644 --- a/src/freshdata/steps/dtypes.py +++ b/src/freshdata/steps/dtypes.py @@ -329,22 +329,35 @@ def _has_relative_date_word(nonnull: pd.Series) -> bool: #: A short-form numeric date whose first two fields could each be a month. -_AMBIGUOUS_DATE = re.compile(r"^\s*(\d{1,2})[-/.](\d{1,2})[-/.](\d{2,4})\s*$") +#: A short numeric date token, found anywhere in the value. The day/month +#: ambiguity lives entirely in this token -- no time, weekday, AM/PM marker or +#: UTC offset around it settles which part is the month -- so it is *searched +#: for*, not matched against the whole string. That makes the detector fail +#: closed: whole-string matching needs a list of every decoration pandas will +#: parse around a date, and each one missing from it is a date read silently +#: in the wrong order. The first version was anchored at ``$`` straight after +#: the year, and ``"05/01/2021 00:00"`` slipped past the ``dayfirst="auto"`` +#: quarantine to be read as 1 May. The digit lookarounds keep a token from +#: starting or ending mid-number (``105/01/2021``, ``05/01/2021123``, ISO). +_AMBIGUOUS_DATE = re.compile(r"(? bool: - """True for a short-form numeric date whose day-first and month-first - readings are *both* valid and differ (``"01/02/2023"``). Day == month + """True when the value holds a short-form numeric date whose day-first and + month-first readings are *both* valid and differ (``"01/02/2023"``), with + or without a time, weekday or offset around it. Day == month (``"01/01/2023"``) reads the same either way, so it is not ambiguous.""" if not isinstance(value, str): return False - m = _AMBIGUOUS_DATE.match(value) - if m is None: - return False - first, second = int(m.group(1)), int(m.group(2)) - return first != second and 1 <= first <= 12 and 1 <= second <= 12 + # Any ambiguous date token makes the value ambiguous: in "13/01/2021 - + # 05/01/2021" the certain first date says nothing about the second. + for m in _AMBIGUOUS_DATE.finditer(value): + first, second = int(m.group(1)), int(m.group(2)) + if first != second and 1 <= first <= 12 and 1 <= second <= 12: + return True + return False def _unresolvable_date_values(nonnull: pd.Series, config: CleanConfig) -> set[str]: diff --git a/tests/test_dayfirst_auto_timed.py b/tests/test_dayfirst_auto_timed.py new file mode 100644 index 0000000..b9aa68f --- /dev/null +++ b/tests/test_dayfirst_auto_timed.py @@ -0,0 +1,108 @@ +"""A time of day must not turn "refuse to guess" into "guess month-first". + +``dayfirst="auto"`` (the default) promises that day/month-ambiguous values +are quarantined for review, never read in an inferred order (``config.py``). +The detector matched only a bare ``DD/MM/YYYY``: ``_AMBIGUOUS_DATE`` was +anchored at ``$`` straight after the year, so ``"05/01/2021 00:00"`` did not +match, was not quarantined, and reached the parser with ``dayfirst=False`` -- +read as 1 May, silently, reported as a plain ``converted to datetime64``. +Appending a time of day flipped the library's behaviour on the same date. + +A first fix widened the anchor to allow a list of time suffixes. Six more +shapes still slipped through it (a comma before the time, a weekday prefix, +a trailing period, ``10h``, ``at 10:00``), because any list of decorations is +incomplete. The detector now searches for the date token anywhere in the +value, so it fails closed: the worst case is over-quarantine, which is +reviewable, never a silent guess. + +The finance domain validator already treats these values as ambiguous (its +own pattern ends ``(?!\\d)``); this brings the core dtype step in line. +""" +from __future__ import annotations + +import pandas as pd +import pytest + +import freshdata as fd +from freshdata.steps.dtypes import _ambiguous_day_month + +#: Every value has both tokens <= 12 and day != month: genuinely ambiguous. +DATES = ["05/01/2021", "06/02/2021", "07/03/2021", "08/04/2021"] +#: Decorations pandas parses around a date. The first nine were the obvious +#: ones; the last six defeated a first fix that listed allowed suffixes +#: instead of searching for the date token -- which is why the detector now +#: searches. ``{}`` is where the date goes. +SHAPES = [ + "{} 00:00", "{} 00:00:00", "{} 00:00:00.123", "{} 3:30 PM", "{} 3:30PM", + "{} 00:00+01:00", "{} 00:00 +0100", "{} 00:00Z", "{}T00:00", + "{}, 10:00", "Mon {}", "Mon {} 10:00", "{}.", "{} 10h", "{} at 10:00", +] + + +def _clean(values, **options): + return fd.clean(pd.DataFrame({"t": values * 6}), verbose=False, **options) + + +def _is_month_first_guess(col: pd.Series) -> bool: + first = col.dropna().iloc[0] if col.notna().any() else None + return hasattr(first, "month") and first.month == 5 and first.day == 1 + + +@pytest.mark.parametrize("shape", SHAPES) +def test_decoration_around_a_date_does_not_defeat_the_auto_quarantine(shape): + """A decorated column must be treated exactly as the bare one is: not guessed.""" + bare = _clean(DATES).data["t"] + decorated = _clean([shape.format(d) for d in DATES]).data["t"] + + assert not _is_month_first_guess(bare), "control: bare dates are never guessed" + assert not _is_month_first_guess(decorated), ( + f"{shape.format(DATES[0])!r} was silently read as 1 May under dayfirst='auto'" + ) + assert str(decorated.dtype) == str(bare.dtype) + + +def test_an_unambiguous_timed_value_still_parses(): + """13 cannot be a month, so ``13/01/2021 10:00`` is certain: 13 January.""" + values = ["13/01/2021 10:00", "14/02/2021 11:00", "15/03/2021 12:00", "16/04/2021 13:00"] + col = _clean(values).data["t"] + assert str(col.dtype).startswith("datetime64") + assert col.iloc[0] == pd.Timestamp(2021, 1, 13, 10, 0) + + +@pytest.mark.parametrize("dayfirst,month,day", [(True, 1, 5), (False, 5, 1)]) +def test_an_explicit_dayfirst_still_resolves_timed_values(dayfirst, month, day): + """The quarantine is for "auto" only; an explicit order is honoured.""" + col = _clean([d + " 00:00" for d in DATES], dayfirst=dayfirst).data["t"] + assert str(col.dtype).startswith("datetime64") + assert (col.iloc[0].month, col.iloc[0].day) == (month, day) + + +def test_in_a_mixed_column_only_the_ambiguous_timed_values_are_quarantined(): + """One ``13/01`` must not decide how ``05/01`` is read -- per value, not per column.""" + certain = ["13/01/2021 10:00", "14/02/2021 11:00", "15/03/2021 12:00"] + ambiguous = ["05/01/2021 10:00"] + result = fd.clean(pd.DataFrame({"t": certain * 10 + ambiguous}), verbose=False) + col = result.data["t"] + report = result.report() + + assert str(col.dtype).startswith("datetime64") + assert col.iloc[0] == pd.Timestamp(2021, 1, 13, 10, 0) + assert pd.isna(col.iloc[-1]), "the ambiguous value must be held, not read as 1 May" + assert "05/01/2021 10:00" in report.coerced_cells.get("t", {}).values() + notes = [a.description for a in report.actions if a.column == "t"] + assert any("day/month-ambiguous" in n for n in notes), notes + + +def test_a_range_is_ambiguous_when_its_second_date_is(): + """A certain first date must not vouch for an ambiguous second one.""" + assert _ambiguous_day_month("13/01/2021 - 05/01/2021") + assert not _ambiguous_day_month("13/01/2021 - 25/01/2021") + + +@pytest.mark.parametrize("value", [ + "2021-05-01", "2021-05-01 10:00:00", "2021-05-01T10:00:00+01:00", "20210501", + "105/01/2021", "05/01/2021123", "13/01/2021 00:00", "01/01/2021 00:00", +]) +def test_the_search_does_not_invent_ambiguity(value): + """ISO, compact and digit-bounded lookalikes, and genuinely certain dates.""" + assert not _ambiguous_day_month(value)