-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
BUG: Fixes Incompatible return value type in pandas/core/generic.py #52897
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
bunardsheng
commented
Apr 24, 2023
•
edited
Loading
edited
- Contributes to issue TYP: investigate/fix ignored mypy errors #37715 (TYP: investigate/fix ignored mypy errors #37715)
- Casts 64-bit signed integer to int value
- Passes unit tests through PyTest
- Added tests to respective folders within tests/series and tests/frame subdirectory
- Removed ignore comments
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.
While subtle, this changes from a NumPy int to a Python int. I think this should be noted in the whatsnew as a bugfix.
Are the changes to pandas_web intentional?
My mistake, I will revert the changes as the changes in web pertain to a separate issue. |
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.
Looks good! A couple of nitpicks and I think this should get an entry in the whatsnew for 2.1.0
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.
All looks great - I think it just needs a note in the whatsnew under Bug fixes
in the Numeric
section.
The entry has been added to the whatsnew. |
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.
I like the change :)
Even though it is a small change it might break code for people who explicitly converted it to int
using Series.size.item()
. I hope this doesn't require a deprecation cycle @jbrockmendel
No objections here |
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
@bunardsheng - conflict needs to be resolved and we're good to go. |
All conflicts should be resolved. Can this PR be merged now, and should I close the PR? |
Thanks @bunardsheng! |
…ndas-dev#52897) Co-authored-by: HappyHorse <[email protected]>
…ndas-dev#52897) Co-authored-by: HappyHorse <[email protected]>