-
-
Notifications
You must be signed in to change notification settings - Fork 145
feat(series): arithmetic sub #1312
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
base: main
Are you sure you want to change the base?
Conversation
1bcf162
to
c9c46b0
Compare
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.
In #1311 you added support for bool
in addition, so don't you want to include the bool
tests for subtraction as well?
It's really nice that the pattern on these operators is similar so it seems pretty straightforward to get them working.
pd.Series([True]) - pd.Series([True]) # TypeError: numpy boolean subtract, the `-` operator, is not supported, use the bitwise_xor, the `^` operator, or the logical_xor function instead. |
But they can be subtracted from non-bools: pd.Series([3,4,5])-pd.Series([True, False, True])
0 2
1 4
2 4
dtype: int64 So I would think you would want to add those to the tests. |
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.
Still missing some tests for completeness.
check(assert_type(left - i, "pd.Series[int]"), pd.Series, np.integer) | ||
check(assert_type(left - f, "pd.Series[float]"), pd.Series, np.floating) | ||
check(assert_type(left - c, "pd.Series[complex]"), pd.Series, np.complexfloating) |
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.
You have to add left-b
to the tests (and similar below) and throughout this file
b, i, f, c = True, 1, 1.0, 1j | ||
|
||
check(assert_type(left - i, "pd.Series[int]"), pd.Series, np.integer) | ||
check(assert_type(left - f, "pd.Series[float]"), pd.Series, np.floating) |
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.
You need to add the tests for b
in here and throughout this file in each of the test functions.
This is inspired by #1274 and a follow-up for #1275.
assert_type()
to assert the type of any return value(I thought cmp0xff#1 would automatically redirect to pandas-dev/pandas-stubs, which did not happen)