-
-
Notifications
You must be signed in to change notification settings - Fork 18.2k
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
Change default of float_precision for read_csv and read_table to "high" #36228
Conversation
If there is no performance difference should we not just get rid of the legacy parsing altogether? |
My only concern here is that maybe someone has code that inadvertently depends on it, so we have to keep it in there for some form of compatibility. I do think we could deprecate the legacy parsing |
yep that sounds fine to leave th option just update the doc string to indicate |
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
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.
minor comments
@jreback added the check for an invalid |
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.
great @Dr-Irv very minor comment can be addressed in a followon (if needed)
@@ -2299,6 +2299,7 @@ def TextParser(*args, **kwds): | |||
values. The options are None for the ordinary converter, | |||
'high' for the high-precision converter, and 'round_trip' for the | |||
round-trip converter. | |||
.. versionchanged:: 1.2 |
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.
check that tis renders ok, I think need a blank line after
tests/io/parser/test_c_parser.py
to make sure all 4 options are testedtests/io/parser/test_c_parser.py:test_high_is_default
black pandas
git diff upstream/master -u -- "*.py" | flake8 --diff
See discussion at bottom of #36149 for the performance tests. Added
float_precision="legacy"
so people can pick up the old parser. Can't change default to "high" because of incompatibility with python parser