-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
MAINT: rename IOError -> OSError #43366
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.
Thank you! The changes look good to me.
@mwtoews can you add a whatsnew note in 1.4., I/O Bug fix section is fine, just noting that we changed this. (and ref this issue). pls merge master and ping on green. |
972495b
to
470a1f9
Compare
470a1f9
to
35d52ff
Compare
@jreback added a whatsnew entry to describe the user-visible change, let me know if it suites or needs rephrasing. I don't understand the Azure "The operation was canceled" CI failure. |
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 you resolve conflicts? Don't worry about the timeouts , they're flaky.
4012e95
to
8097149
Compare
8097149
to
1ab776e
Compare
|
… or file-like object
deedac0
to
4f019ad
Compare
@mwtoews - can you take a look at the test failures? |
d69f47b
to
67cba78
Compare
67cba78
to
5f99a9b
Compare
The third commit in this PR fixes the excel readers, which allow bytes for the first parameter (e.g. pandas.read_excel). Since I'd appreciate a review from anyone familiar with |
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.
cc @rhshadrach
It seems that support for reading bytes for the excel readers was added with #30519 Also, feel free to re-title and re-classify this PR, as the scope has shifted since initially submitted. |
no its fine, just the question on where the bytes are wrapped is still likely hit |
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.
excel changes lgtm - verified tests would fail without the changes in excel/_base
@github-actions pre-commit |
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. and thanks for the PR (I took care of the pre-commit issue for you).
P.S. You don't have to force-push/squash your commits if you don't want to. We squash on merge so it's fine.
thanks @mwtoews |
Since Python 3.3, IO and OS exceptions were re-worked as per PEP 3151. The new main base class is OSError, with several other exceptions merged, including IOError (i.e.
assert IOError is OSError
).This PR changes many of the old alias to OSError, with a few exceptions:
pandas/_libs/parsers.pyx
, this is not an OSError, as there is no issue reading the file. The exception is reclassified as a TypeError.Are any of these changes worthy of a whatsnew entry?