Skip to content

feat: improve date parsing - #245

Open
ThibaudDauce wants to merge 18 commits into
mainfrom
improve_date_parsing
Open

feat: improve date parsing#245
ThibaudDauce wants to merge 18 commits into
mainfrom
improve_date_parsing

Conversation

@ThibaudDauce

@ThibaudDauce ThibaudDauce commented Apr 8, 2026

Copy link
Copy Markdown
Contributor
Fichier inspection csv to db total
main branche Δ main branche Δ main branche Δ
ValeursFoncieres-2024.txt 44,2 44,3 +0 % 50,1 31,6 −37 % 94,3 75,9 −20 %
ValeursFoncieres-2025-S1.txt 16,4 16,5 +0 % 20,2 12,7 −37 % 36,7 29,2 −20 %
MN_07 (sans date) 30,5 28,9 −5 % 9,9 10,1 +2 % 40,3 39,0 −3 %
joconde.csv 106,8 104,2 −2 % 22,6 18,5 −18 % 129,4 122,8 −5 %
irve.csv 7,9 6,7 −15 % 12,6 5,7 −55 % 20,6 12,4 −40 %

Some analyses will be invalidated and replayed after the merge. Values dateutil used to read as dates and that we don't detect anymore, on the files I have locally:

Fichier Colonne Valeurs Exemple
joconde.csv Mesures 693 D. 2.9 ; E. 0.6
joconde.csv Numero_inventaire 270 M 1089
joconde.csv Millesime_de_creation 76 134,138
irve.csv consolidated_latitude 1700 48.825613
irve.csv telephone_operateur 12 0680428426
MN_07 AAAAMMJJHHMN 2000 202501010000

Everything but the last one is noise dateutil was finding in phone numbers, latitudes and museum inventory numbers. AAAAMMJJHHMN is a real one: it's now datetime_naive where it was date, so the column type changes in the db (it holds hours and minutes that the date type was dropping).

@ThibaudDauce
ThibaudDauce force-pushed the improve_date_parsing branch from 0f8d074 to ab90678 Compare April 8, 2026 11:59
@estellebertrand

Copy link
Copy Markdown

A necessary change! The logic looks good, but I need to take more time to review all changes more carefully :)

@bolinocroustibat bolinocroustibat left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, with two remarks

Comment thread csv_detective/formats/date.py
Comment thread tests/test_fields.py

@estellebertrand estellebertrand left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - still reading it all to ensure no regression

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here, unsure why do we drop this format ? Because not an known separate format and this can be included in date.py just as a type of date format like any other ?
[the comment was for the deleted date_fr file, not appearing on the right one here + this has been managed since by commit 48219e8]

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.

3 participants