From db2c6419b5f5a9810b468758a5feaf5c07e95f68 Mon Sep 17 00:00:00 2001 From: "HE, Tao" Date: Thu, 7 Mar 2019 19:39:52 +0800 Subject: [PATCH 1/4] Fix _binop for operators which has more than one returns (divmod). Signed-off-by: HE, Tao --- doc/source/whatsnew/v0.24.2.rst | 1 + pandas/core/series.py | 22 ++++++++++++++++------ pandas/tests/series/test_operators.py | 14 ++++++++++++++ 3 files changed, 31 insertions(+), 6 deletions(-) diff --git a/doc/source/whatsnew/v0.24.2.rst b/doc/source/whatsnew/v0.24.2.rst index 4ca9d57f3a2e5..8e905303032c9 100644 --- a/doc/source/whatsnew/v0.24.2.rst +++ b/doc/source/whatsnew/v0.24.2.rst @@ -31,6 +31,7 @@ Fixed Regressions - Fixed regression in ``IntervalDtype`` construction where passing an incorrect string with 'Interval' as a prefix could result in a ``RecursionError``. (:issue:`25338`) - Fixed regression in :class:`Categorical`, where constructing it from a categorical ``Series`` and an explicit ``categories=`` that differed from that in the ``Series`` created an invalid object which could trigger segfaults. (:issue:`25318`) - Fixed pip installing from source into an environment without NumPy (:issue:`25193`) +- Fixed bug in :meth:`Series._binop` to handle binary operators that returns more than one :class:`Series` correctly (:issue:`25557`) .. _whatsnew_0242.enhancements: diff --git a/pandas/core/series.py b/pandas/core/series.py index 03fc26efa4516..3ad9d32c8d73b 100644 --- a/pandas/core/series.py +++ b/pandas/core/series.py @@ -2502,6 +2502,15 @@ def _binop(self, other, func, level=None, fill_value=None): ------- Series """ + + def mk_ret(result, new_index, name): + 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): + 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): """ diff --git a/pandas/tests/series/test_operators.py b/pandas/tests/series/test_operators.py index b2aac441db195..bfc88698e9717 100644 --- a/pandas/tests/series/test_operators.py +++ b/pandas/tests/series/test_operators.py @@ -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']) + b = Series([2, np.nan, 1, np.nan], index=['a', 'b', 'd', 'e']) + + r1 = divmod(a, b) + 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__ From 87d163686a7c61bf327115949092f7c80732de55 Mon Sep 17 00:00:00 2001 From: "HE, Tao" Date: Thu, 7 Mar 2019 21:28:53 +0800 Subject: [PATCH 2/4] Do __finalize__ in _construct_result and refactor previous solution. Signed-off-by: HE, Tao --- doc/source/whatsnew/v0.24.2.rst | 2 +- pandas/core/ops.py | 12 +++++++----- pandas/core/series.py | 19 +++++-------------- pandas/tests/series/test_operators.py | 1 + 4 files changed, 14 insertions(+), 20 deletions(-) diff --git a/doc/source/whatsnew/v0.24.2.rst b/doc/source/whatsnew/v0.24.2.rst index 8e905303032c9..4dcc9377a0e10 100644 --- a/doc/source/whatsnew/v0.24.2.rst +++ b/doc/source/whatsnew/v0.24.2.rst @@ -31,7 +31,7 @@ Fixed Regressions - Fixed regression in ``IntervalDtype`` construction where passing an incorrect string with 'Interval' as a prefix could result in a ``RecursionError``. (:issue:`25338`) - Fixed regression in :class:`Categorical`, where constructing it from a categorical ``Series`` and an explicit ``categories=`` that differed from that in the ``Series`` created an invalid object which could trigger segfaults. (:issue:`25318`) - Fixed pip installing from source into an environment without NumPy (:issue:`25193`) -- Fixed bug in :meth:`Series._binop` to handle binary operators that returns more than one :class:`Series` correctly (:issue:`25557`) +- Fixed bug in :meth:`Series.divmod` and :meth:`Series.rdivmod` (:issue:`25557`) .. _whatsnew_0242.enhancements: diff --git a/pandas/core/ops.py b/pandas/core/ops.py index dbdabecafae3a..f85b16467f7f6 100644 --- a/pandas/core/ops.py +++ b/pandas/core/ops.py @@ -1494,18 +1494,20 @@ 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.name = name + out.__finalize__(left) + if name is None: + 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), ) diff --git a/pandas/core/series.py b/pandas/core/series.py index 3ad9d32c8d73b..d08cabaacca42 100644 --- a/pandas/core/series.py +++ b/pandas/core/series.py @@ -2503,14 +2503,6 @@ def _binop(self, other, func, level=None, fill_value=None): Series """ - def mk_ret(result, new_index, name): - 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') @@ -2527,14 +2519,13 @@ def mk_ret(result, new_index, name): with np.errstate(all='ignore'): result = func(this_vals, other_vals) + name = ops.get_op_result_name(self, other) - if isinstance(result, tuple): - final_result = () - for r in result: - final_result = (*final_result, mk_ret(r, new_index, name)) + if func.__name__ in ['divmod', 'rdivmod']: + ret = ops._construct_divmod_result(self, result, new_index, name) else: - final_result = mk_ret(result, new_index, name) - return final_result + ret = ops._construct_result(self, result, new_index, name) + return ret def combine(self, other, func, fill_value=None): """ diff --git a/pandas/tests/series/test_operators.py b/pandas/tests/series/test_operators.py index bfc88698e9717..16bd0efdf272e 100644 --- a/pandas/tests/series/test_operators.py +++ b/pandas/tests/series/test_operators.py @@ -742,6 +742,7 @@ def test_op_duplicate_index(self): assert_series_equal(result, expected) def test_divmod(self): + # GH25557 a = Series([1, 1, 1, np.nan], index=['a', 'b', 'c', 'd']) b = Series([2, np.nan, 1, np.nan], index=['a', 'b', 'd', 'e']) From e15cf7273016e33a54d4ca30e8a202e5c5ac8b4e Mon Sep 17 00:00:00 2001 From: "HE, Tao" Date: Tue, 12 Mar 2019 13:48:14 +0800 Subject: [PATCH 3/4] Revise the doc, refactor test case and remove the incorrect `if` condition. Signed-off-by: HE, Tao --- doc/source/whatsnew/v0.25.0.rst | 3 +-- pandas/core/ops.py | 3 +-- pandas/tests/series/test_operators.py | 18 +++++++++--------- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/doc/source/whatsnew/v0.25.0.rst b/doc/source/whatsnew/v0.25.0.rst index 2755180f06478..e6bbf046f61e8 100644 --- a/doc/source/whatsnew/v0.25.0.rst +++ b/doc/source/whatsnew/v0.25.0.rst @@ -122,8 +122,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`) -- Fixed bug in :meth:`Series.divmod` and :meth:`Series.rdivmod` (:issue:`25557`) -- +- 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`) Categorical ^^^^^^^^^^^ diff --git a/pandas/core/ops.py b/pandas/core/ops.py index c2491ec23866d..efcc39f116fae 100644 --- a/pandas/core/ops.py +++ b/pandas/core/ops.py @@ -1661,8 +1661,7 @@ def _construct_result(left, result, index, name, dtype=None): """ out = left._constructor(result, index=index, dtype=dtype) out.__finalize__(left) - if name is None: - out.name = name + out.name = name return out diff --git a/pandas/tests/series/test_operators.py b/pandas/tests/series/test_operators.py index 16bd0efdf272e..2f96fe906d980 100644 --- a/pandas/tests/series/test_operators.py +++ b/pandas/tests/series/test_operators.py @@ -746,15 +746,15 @@ def test_divmod(self): a = Series([1, 1, 1, np.nan], index=['a', 'b', 'c', 'd']) b = Series([2, np.nan, 1, np.nan], index=['a', 'b', 'd', 'e']) - r1 = divmod(a, b) - 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]) + 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): From 3bde35323215e0af3b7006e6bd8bd5567098cf46 Mon Sep 17 00:00:00 2001 From: "HE, Tao" Date: Wed, 20 Mar 2019 11:35:30 +0800 Subject: [PATCH 4/4] Fix. Signed-off-by: HE, Tao --- pandas/core/ops.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/core/ops.py b/pandas/core/ops.py index efcc39f116fae..bd45003c154a4 100644 --- a/pandas/core/ops.py +++ b/pandas/core/ops.py @@ -1660,7 +1660,7 @@ 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) + out = out.__finalize__(left) out.name = name return out