Skip to content

Conversation

@loicdiridollou
Copy link
Member

I have updated the code in the pandas/io/parsers/readers.py since if we pased parse_dates=False, it would still change it to True.

@loicdiridollou loicdiridollou changed the title Parse date bug BUG: parse_dates=False while passing date_parser tries to use date parser Nov 24, 2021
Copy link
Member

@phofl phofl left a 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

filepath_or_buffer: FilePath | ReadCsvBuffer[bytes] | ReadCsvBuffer[str], kwds
):
"""Generic reader of line files."""
if kwds.get("date_parser", None) is not None:
Copy link
Member

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 
Copy link
Member Author

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)

@loicdiridollou loicdiridollou requested a review from phofl November 24, 2021 17:22

testdata = StringIO(
"""time e n h
41047.00 -98573.7297 871458.0640 389.0089
Copy link
Member

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

Copy link
Member Author

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)

@loicdiridollou loicdiridollou requested a review from phofl November 24, 2021 18:56
@jreback jreback added this to the 1.4 milestone Nov 25, 2021
@jreback jreback added Bug IO CSV read_csv, to_csv labels Nov 25, 2021
Copy link
Contributor

@jreback jreback left a 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

@phofl phofl merged commit 96eb4c9 into pandas-dev:master Nov 25, 2021
@phofl
Copy link
Member

phofl commented Nov 25, 2021

@loicdiridollou loicdiridollou deleted the parse_date_bug branch November 25, 2021 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug IO CSV read_csv, to_csv

3 participants