-
-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
fix #54564 #54567
fix #54564 #54567
Conversation
Note that I had to add a new |
doc/source/whatsnew/v2.1.0.rst
Outdated
@@ -749,6 +749,7 @@ I/O | |||
- Bug in :func:`json_normalize`, fix json_normalize cannot parse metadata fields list type (:issue:`37782`) | |||
- Bug in :func:`read_csv` where it would error when ``parse_dates`` was set to a list or dictionary with ``engine="pyarrow"`` (:issue:`47961`) | |||
- Bug in :func:`read_csv`, with ``engine="pyarrow"`` erroring when specifying a ``dtype`` with ``index_col`` (:issue:`53229`) | |||
- Bug in :func:`read_excel`, with ``engine="xlrd"`` (``xls`` files) erroring when file contains NaNs/Infs (:issue:`54564`) |
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.
@mroeschke - should this be going into 2.2 at this point?
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.
Yeah probably. (Maybe "higher impact" bugs can go in 2.1 still)
pandas/io/excel/_xlrd.py
Outdated
# GH54564 - if the cell contents are NaN/Inf, we get an exception; | ||
# that is just another case where we don't want to convert. | ||
# The exception filter is quite general on purpose: whenever | ||
# the cell content cannot be converted to int - just don't. |
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 appears to be pretty verbose to me. Would something like "Don't convert NaN/Inf values" suffice?
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; cc @mroeschke
Looks like the added test needs to handle Window's 32 bit default |
Yea, just noticed that. @burnpanck - NumPy has a bit of an oddity where ints default to 32 bit on Windows (they're fixing this in 2.0 I believe). Can you specify https://github.com/pandas-dev/pandas/actions/runs/5902921221/job/16011817499?pr=54567#step:5:1041 Edit: I forgot this has a mix of columns. You can use astype on the integer one. |
Thanks @burnpanck |
doc/source/whatsnew/v2.1.0.rst
file: -> may this fix make it into a potential v2.0.4 instead?