Skip to content

fix(dtypes): a time of day no longer defeats the dayfirst="auto" quarantine - #506

Merged
kevincostner17 merged 1 commit into
mainfrom
fix/dayfirst-auto-timed
Sep 28, 2026
Merged

kevincostner17 merged 1 commit into
mainfrom
fix/dayfirst-auto-timed

Conversation

@kevincostner17

@kevincostner17 kevincostner17 commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Bug

Under the default dayfirst="auto", day/month-ambiguous dates are documented to be quarantined for review, never read in an inferred order (config.py). A bare date is. Add a time of day and it isn't:

vals = ["05/01/2021", "06/02/2021", "07/03/2021", "08/04/2021"] * 6
fd.clean(pd.DataFrame({"t": vals})).data["t"]                            # object — not guessed ✓
fd.clean(pd.DataFrame({"t": [v + " 00:00" for v in vals]})).data["t"][0]  # Timestamp('2021-05-01') ✗

The timed column is read month-first, and the report says only converted to datetime64[ns]: no quarantine, no ambiguity note, no warning. Every value is ambiguous (both tokens ≤ 12), the exact case "auto" exists to protect. On a day-first export, every date lands in the wrong month. This affects every supported pandas version.

Cause

steps/dtypes.py:

_AMBIGUOUS_DATE = re.compile(r"^\s*(\d{1,2})[-/.](\d{1,2})[-/.](\d{2,4})\s*$")

The pattern is anchored at $ straight after the year, so any time suffix stops the match. The value is then never quarantined and reaches _parse_datetime with dayfirst = config.dayfirst if isinstance(config.dayfirst, bool) else False, so "auto" silently becomes month-first for any value the detector misses.

Fix

Search for the numeric date token anywhere in the value instead of matching the whole string; a value is ambiguous if any of its tokens is:

_AMBIGUOUS_DATE = re.compile(r"(?<!\d)(\d{1,2})[-/.](\d{1,2})[-/.](\d{2,4})(?!\d)")

No time, weekday, AM/PM marker or UTC offset around the token says which part is the month.

Why search rather than widen the anchor. A first version widened the anchor to allow a list of time suffixes. It passed its own tests, but a probe of shapes outside that list found six still read month-first: "05/01/2021, 10:00", "Mon 05/01/2021", "Mon 05/01/2021 10:00", "05/01/2021.", "05/01/2021 10h", "05/01/2021 at 10:00". Any list of decorations pandas will parse is incomplete, so a whole-string detector fails open, and each missing shape becomes a silent wrong date. Searching fails closed: the worst case is over-quarantine, which is reviewable because the original is kept in coerced_cells.

The finance domain validator already treats timed values as ambiguous (its own pattern ends in (?!\d)), so the core dtype step now agrees with it.

Tests

tests/test_dayfirst_auto_timed.py, 28 tests:

  • 15 decorated shapes, including the six above: each must behave exactly like the bare column (not guessed, same dtype)
  • unambiguous timed values (13/01/2021 10:00) still parse
  • explicit dayfirst=True / False still resolve timed values both ways
  • in a mixed column only the ambiguous value is quarantined, with its original kept in coerced_cells and a day/month-ambiguity note
  • a range whose second date is ambiguous counts as ambiguous
  • no invented ambiguity: ISO with and without time/offset, 20210501, 105/01/2021, 05/01/2021123, certain dates

17 fail without the src/ change, on both py3.12 / pandas 2.3.3 and py3.9 / pandas 1.5.3. The 11 that pass either way are controls.

Checked and unaffected: _PARTIAL_ISO_DATE has the same ^…$ anchoring, but "2025-01 10:00" is still left missing, not given an invented day.

Verification

Full suite (-m "not online and not large") on d65e896 (after #504 and #505) + this change:

  • py3.12 / pandas 2.3.3: 7539 passed, 0 failed
  • py3.9 / pandas 1.5.3: 7507 passed, 0 failed

ruff check clean.

Compatibility impact

Output changes for columns of numeric dates with anything around them (a time, weekday, offset or trailing punctuation) where both day and month are ≤ 12, cleaned with the default dayfirst="auto". These values are now quarantined — set to missing, with the original in report.coerced_cells — exactly as the same dates without decoration always were, and a column made entirely of them stays text. Previously they were converted month-first. To convert them, state the order with dayfirst=True or dayfirst=False. Unambiguous values and explicit dayfirst settings are unchanged.

…antine

Under the default dayfirst="auto", day/month-ambiguous dates are documented
to be quarantined for review, never read in an inferred order (config.py).
A bare "05/01/2021" was. But "05/01/2021 00:00" was read as 1 May -- silently,
reported as a plain "converted to datetime64[ns]", no quarantine, no
ambiguity note, no warning. On a day-first export every date landed in the
wrong month. Affects every supported pandas version.

_AMBIGUOUS_DATE was anchored at $ straight after the year, so any time
suffix stopped the match; the value then reached the parser, where "auto"
becomes month-first.

The detector now searches for the numeric date token anywhere in the value
rather than matching the whole string, and a value is ambiguous if any of
its tokens is. A first version that listed allowed time suffixes still let
six shapes through ("05/01/2021, 10:00", "Mon 05/01/2021", "05/01/2021.",
"05/01/2021 10h", "05/01/2021 at 10:00", ...): any list of decorations is
incomplete, so a whole-string detector fails open. Searching fails closed --
the worst case is over-quarantine, which is reviewable.

The finance domain validator already treated timed values as ambiguous; the
core dtype step now agrees with it.

tests/test_dayfirst_auto_timed.py: 28 tests; 17 fail without this change on
both py3.12/pandas 2.3.3 and py3.9/pandas 1.5.3.
@coderabbitai

coderabbitai Bot commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 7be42c03-2838-4970-8724-612b77f6eaea


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown

FreshData benchmark report — performance

  • freshdata: ?
  • python: ?
  • platform: ?
fixture n_rows n_cols p50 s p95 s peak MB repair % false-repair % preserve % trust monotonic export %

Authored-code reduction (Metric 6)

@kevincostner17
kevincostner17 merged commit 28ce1c3 into main Sep 28, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant