Improve date/time hint handling for Pandas based on better testing #148
Conversation
Based on the new testing approach in #144, this PR adds component tests for date/time types while loading/exporting CSV files in Pandas. Along the way some places for improvement were found!
| 'YYYY-MM-DD': '%Y-%m-%d', | ||
| 'MM/DD/YY': '%m/%d/%Y', | ||
| 'DD/MM/YY': '%d/%m/%Y', |
There was a problem hiding this comment.
These were wrong! %Y is a four digit year, %y is a two digit year.
| @@ -39,16 +38,19 @@ | |||
| # account for MM/DD time. | |||
| # | |||
| # https://github.com/bluelabsio/records-mover/issues/75 | |||
| # | |||
| python_date_format_from_hints: Dict[Union[HintDateFormat, Literal['DD/MM/YY']], str] = { | |||
| python_date_format_from_hints: Dict[HintDateFormat, str] = { | |||
There was a problem hiding this comment.
Since this is now a str alias, no reason to add more things to it.
| else: | ||
| cant_handle_hint(fail_if_cant_handle_hint, 'dateformat', hints) | ||
| quiet_remove(unhandled_hints, 'dateformat') | ||
|
|
||
| # pandas can't seem to export a date and time together :( |
There was a problem hiding this comment.
This comment isn't right; it certainly can. What it can't do is export a time without a date, or a date without a time. That's what we have prep_for_csv.py for.
…om:bluelabsio/records-mover into loosen_date_requirements_4
| } | ||
|
|
||
| python_time_format_from_hints: Dict[HintTimeOnlyFormat, str] = { | ||
| 'HH24:MI:SS': '%H:%M:%S', | ||
| 'HH12:MI AM': '%I:%M:%S %p', | ||
| 'HH:MI:SS': '%H:%M:%S', | ||
| 'HH12:MI AM': '%I:%M %p', |
There was a problem hiding this comment.
We were adding seconds here against what the user was telling us. We have a similar bug with datetime/datetimetz types - see #143
Once we have more solid tests here we can go back and generalize to_csv_options() and fix those along the way - to fix it with the current design would be basically remove (mostly-working) functionality.
Co-authored-by: Chris Wegrzyn <[email protected]>
Based on the new testing approach in #144, this PR adds component tests for date/time types while loading/exporting CSV files in Pandas.
Along the way some places for improvement were found!