-
-
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
TST: Don't ignore tolerance for integer series #56724
Conversation
Thanks, can you also update the docstrings? |
done |
Looks like 2 more places need to be updated. pandas/pandas/_testing/asserters.py Lines 718 to 719 in 8b287cf
pandas/pandas/_testing/asserters.py Lines 1124 to 1125 in 8b287cf
|
Oh sorry, I think I got everything now |
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. Thx for the quick fix!
cc @MarcoGorelli if you have any comments as well
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.
little nitpick but looks good to me!
check_exact = ( | ||
is_numeric_dtype(left.dtype) | ||
and not is_float_dtype(left.dtype) | ||
or is_numeric_dtype(right.dtype) | ||
and not is_float_dtype(right.dtype) | ||
) |
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.
nitpick - I can never remember how a logic statement like this will be parsed, fancy making it move explicit with
check_exact = (
(is_numeric_dtype(left.dtype)
and not is_float_dtype(left.dtype))
or (is_numeric_dtype(right.dtype)
and not is_float_dtype(right.dtype))
)
?
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 would make the intendation worse? Not sure how black treats those
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.
up to you, but it's very easy to get these wrong, e.g. https://github.com/pandas-dev/pandas/pull/34334/files
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.
The intents look really off with parenthesis, will merge for now
…nteger series) (#56786) Backport PR #56724: TST: Don't ignore tolerance for integer series Co-authored-by: Patrick Hoefler <[email protected]>
pd.testing.assert_series_equal
break in version 2.2.0rc0 #56646 (Replace xxxx with the GitHub issue number)doc/source/whatsnew/vX.X.X.rst
file if fixing a bug or adding a new feature.cc @lithomas1