Skip to content

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

Merged
merged 7 commits into from
Mar 20, 2019
1 change: 1 addition & 0 deletions doc/source/whatsnew/v0.25.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ Bug Fixes
- Bug in :func:`to_datetime` which would raise an (incorrect) ``ValueError`` when called with a date far into the future and the ``format`` argument specified instead of raising ``OutOfBoundsDatetime`` (:issue:`23830`)
- Bug in an error message in :meth:`DataFrame.plot`. Improved the error message if non-numerics are passed to :meth:`DataFrame.plot` (:issue:`25481`)
- Bug in error messages in :meth:`DataFrame.corr` and :meth:`Series.corr`. Added the possibility of using a callable. (:issue:`25729`)
- Bug in :meth:`Series.divmod` and :meth:`Series.rdivmod` which would raise an (incorrect) ``ValueError`` rather than return a pair of :class:`Series` object as result (:issue:`25557`)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

object -> objects as a result

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. I have moved this line to the Numeric section.


Categorical
^^^^^^^^^^^
Expand Down
9 changes: 5 additions & 4 deletions pandas/core/ops.py
Original file line number Diff line number Diff line change
Expand Up @@ -1660,18 +1660,19 @@ def _construct_result(left, result, index, name, dtype=None):
not be enough; we still need to override the name attribute.
"""
out = left._constructor(result, index=index, dtype=dtype)

out.__finalize__(left)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this be out = out.__finalize__(left)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Thanks!

out.name = name
return out


def _construct_divmod_result(left, result, index, name, dtype=None):
"""divmod returns a tuple of like indexed series instead of a single series.
"""
constructor = left._constructor
return (
constructor(result[0], index=index, name=name, dtype=dtype),
constructor(result[1], index=index, name=name, dtype=dtype),
_construct_result(left, result[0], index=index, name=name,
dtype=dtype),
_construct_result(left, result[1], index=index, name=name,
dtype=dtype),
)


Expand Down
13 changes: 7 additions & 6 deletions pandas/core/series.py
Original file line number Diff line number Diff line change
Expand Up @@ -2502,6 +2502,7 @@ def _binop(self, other, func, level=None, fill_value=None):
-------
Series
"""

if not isinstance(other, Series):
raise AssertionError('Other operand must be Series')

Expand All @@ -2518,13 +2519,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 func.__name__ in ['divmod', 'rdivmod']:
ret = ops._construct_divmod_result(self, result, new_index, name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could just return on each statement

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previous review comment suggested me to use _construct_divmod_result.

else:
ret = ops._construct_result(self, result, new_index, name)
return ret

def combine(self, other, func, fill_value=None):
"""
Expand Down
15 changes: 15 additions & 0 deletions pandas/tests/series/test_operators.py
Original file line number Diff line number Diff line change
Expand Up @@ -741,6 +741,21 @@ 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):
# GH25557
a = Series([1, 1, 1, np.nan], index=['a', 'b', 'c', 'd'])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add the GitHub issue number as a comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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'])

result = a.divmod(b)
expected = divmod(a, b)
assert_series_equal(result[0], expected[0])
assert_series_equal(result[1], expected[1])

result = a.rdivmod(b)
expected = divmod(b, a)
assert_series_equal(result[0], expected[0])
assert_series_equal(result[1], expected[1])


class TestSeriesUnaryOps(object):
# __neg__, __pos__, __inv__
Expand Down