-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
BUG: parse_dates=False while passing date_parser tries to use date parser #44599
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add tests. One whatsnew is enough
pandas/io/parsers/readers.py
Outdated
@@ -511,8 +511,14 @@ def _read( | |||
): | |||
"""Generic reader of line files.""" | |||
if kwds.get("date_parser", None) is not None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is hard to read. Please do it somehow like this:
if kwds.get("date_parser", None) is not None and kwds.get("pars_dates", None) is None:
kwds.get["pars_dates"] = True
elif kwds.get("pars_dates", None) is None:
kwds.get["pars_dates"] = False
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct! I have made the changes so now it is more readable, added a test to see if the parser is not called when we set parse_dates
to False
.
Some tests are failing for an obscure reason as they are not related to the code change (and they are also failing on other builds)
|
||
testdata = StringIO( | ||
"""time e n h | ||
41047.00 -98573.7297 871458.0640 389.0089 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we reduce number of columns and use small ints? This is really hard to read
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, this should be better now. I was just reusing the data from test_read_csv_with_custom_date_parser
(which could also be simplified at a later stage)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. @phofl merge when good
thx @loicdiridollou |
I have updated the code in the
pandas/io/parsers/readers.py
since if we pasedparse_dates=False
, it would still change it to True.