fix: cast dd/mm/yyyy dates day-first (was silently swapping day and month) - #268
fix: cast dd/mm/yyyy dates day-first (was silently swapping day and month)#268estellebertrand wants to merge 1 commit into
Conversation
|
I think this solution will break for US dates in files, (it will take french format for dates that match french format and US format for dates with US format during casting, no?). I've already worked on this in #245 for other reasons, but I can try to incorporate a fix for this problem too (getting the format at detection time from the full file and just using it at casting time without redetecting) |
Yes it will break any US dates that could be matched as a French date. But as of today, from my understanding, the same is true about French dates being interpreted as US dates during casting - and I imagine French dates may be more common in our datasets (?). Though I totally agree that is an issue and should be improved, I opened this issue #269. It looks like your PR #245 is actually implementing what I had in mind ? |
bolinocroustibat
left a comment
There was a problem hiding this comment.
LGTM, thank you for spotting and solving this!
(Tempted to open an issue on date parsers about this, defaulting to month-first is just the worst)
|
Closing, replaced by #245 |
Problem
date._is()acceptsdd/mm/yyyyvalues throughjjmmaaaa_pattern, which is day-firstby construction.
But
date_casting()then converted those values withdateutil.parser.parse(val),falling back on
dateparser.parse(val)- both default to month-first, and neitherwas given a
dayfirst/DATE_ORDERargument.The casting function is used in Hydra (to be confirmed), and every value where both components are
≤ 12 was silently inverted:
The inversion is silent: no error, no warning, and the output is a perfectly valid date.
This issue has been uncovered through this discussion on the platform.
Proposal
Pass
dayfirstto the parsers only for values that matchjjmmaaaa_patternfrom the detection step. Because this pattern requires the second component to be 01–12 and the first to be 01–31, a matching value can only be read as DD then MM.date_castingmoves below the pattern definitions so it can usejjmmaaaa_pattern.Note : Why not just
dayfirst=TrueeverywhereTried first, and it breaks two things:
dateutilappliesdayfirstto the day/month pair regardless of position in regard to year digits, so ISO2024-03-07was parsed as 3 July.DATE_ORDER: DMYmakes thedateparserfallback accept12152003,20031512and02052003, whichdate._test_values[False]explicitly requires to be rejected.Scoping the flag to the pattern avoids both.
Impact on existing data
Behaviour-changing for downstream consumers. Dates already stored by hydra for CSV resources with ambiguous
dd/mm/yyyyvalues are wrong and would need reprocessing.