-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
BUG: Fix _binop for operators for serials which has more than one returns (divmod/rdivmod). #25588
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
Changes from 1 commit
db2c641
87d1636
81e87bc
e15cf72
5d40c93
bce25c9
3bde353
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2502,6 +2502,15 @@ def _binop(self, other, func, level=None, fill_value=None): | |
------- | ||
Series | ||
""" | ||
|
||
def mk_ret(result, new_index, name): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you reuse anything from |
||
ret = self._constructor(result, index=new_index, name=name) | ||
ret.__finalize__(self) | ||
if name is None: | ||
# When name is None, __finalize__ overwrites current name | ||
ret.name = None | ||
return ret | ||
|
||
if not isinstance(other, Series): | ||
raise AssertionError('Other operand must be Series') | ||
|
||
|
@@ -2519,12 +2528,13 @@ def _binop(self, other, func, level=None, fill_value=None): | |
with np.errstate(all='ignore'): | ||
result = func(this_vals, other_vals) | ||
name = ops.get_op_result_name(self, other) | ||
result = self._constructor(result, index=new_index, name=name) | ||
result = result.__finalize__(self) | ||
if name is None: | ||
# When name is None, __finalize__ overwrites current name | ||
result.name = None | ||
return result | ||
if isinstance(result, tuple): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hopefully replace with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for reminding me that two utilities. After There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. in core/ops.py, we call There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. However after There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or should we do There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's ok to do Will do that. |
||
final_result = () | ||
for r in result: | ||
final_result = (*final_result, mk_ret(r, new_index, name)) | ||
else: | ||
final_result = mk_ret(result, new_index, name) | ||
return final_result | ||
|
||
def combine(self, other, func, fill_value=None): | ||
""" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -741,6 +741,20 @@ def test_op_duplicate_index(self): | |
expected = pd.Series([11, 12, np.nan], index=[1, 1, 2]) | ||
assert_series_equal(result, expected) | ||
|
||
def test_divmod(self): | ||
a = Series([1, 1, 1, np.nan], index=['a', 'b', 'c', 'd']) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add the GitHub issue number as a comment. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will do. |
||
b = Series([2, np.nan, 1, np.nan], index=['a', 'b', 'd', 'e']) | ||
|
||
r1 = divmod(a, b) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use the format There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed. |
||
r2 = a.divmod(b) | ||
assert_series_equal(r1[0], r2[0]) | ||
assert_series_equal(r1[1], r2[1]) | ||
|
||
r3 = divmod(b, a) | ||
r4 = a.rdivmod(b) | ||
assert_series_equal(r3[0], r4[0]) | ||
assert_series_equal(r3[1], r4[1]) | ||
|
||
|
||
class TestSeriesUnaryOps(object): | ||
# __neg__, __pos__, __inv__ | ||
|
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.
Reference :class:=
Series.divmod=
. These note are for users, and _binop isn't public.You can remove the "that returns more than one Series correctly" part.
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.
Will refactor this part, thanks!