From 52538fa03a8c9722ab5c86c88419105b6ebfe5a1 Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Mon, 1 Oct 2018 16:51:48 -0500 Subject: [PATCH 01/12] BUG: divmod return type --- doc/source/extending.rst | 15 ++++++++++++--- pandas/core/arrays/base.py | 16 ++++++++++++---- pandas/tests/extension/decimal/array.py | 4 ++++ pandas/tests/extension/test_ops.py | 22 ++++++++++++++++++++++ 4 files changed, 50 insertions(+), 7 deletions(-) create mode 100644 pandas/tests/extension/test_ops.py diff --git a/doc/source/extending.rst b/doc/source/extending.rst index 9422434a1d998..da249cb3592f4 100644 --- a/doc/source/extending.rst +++ b/doc/source/extending.rst @@ -160,9 +160,18 @@ your ``MyExtensionArray`` class, as follows: MyExtensionArray._add_arithmetic_ops() MyExtensionArray._add_comparison_ops() -Note that since ``pandas`` automatically calls the underlying operator on each -element one-by-one, this might not be as performant as implementing your own -version of the associated operators directly on the ``ExtensionArray``. + +.. note:: + + Since ``pandas`` automatically calls the underlying operator on each + element one-by-one, this might not be as performant as implementing your own + version of the associated operators directly on the ``ExtensionArray``. + +This implementation will try to reconstruct a new ``ExtensionArray`` with the +result of the element-wise operation. Whether or not that succeeds depends on +whether the operation returns a result that's valid for the ``ExtensionArray``. +If an ``ExtensionArray`` cannot be reconstructed, a list containing the scalars +returned instead. .. _extending.extension.testing: diff --git a/pandas/core/arrays/base.py b/pandas/core/arrays/base.py index 7bf13fb2fecc0..5a18a1049ee9c 100644 --- a/pandas/core/arrays/base.py +++ b/pandas/core/arrays/base.py @@ -775,10 +775,18 @@ def convert_values(param): res = [op(a, b) for (a, b) in zip(lvalues, rvalues)] if coerce_to_dtype: - try: - res = self._from_sequence(res) - except TypeError: - pass + if op.__name__ in {'divmod', 'rdivmod'}: + try: + a, b = zip(*res) + res = (self._from_sequence(a), + self._from_sequence(b)) + except TypeError: + pass + else: + try: + res = self._from_sequence(res) + except TypeError: + pass return res diff --git a/pandas/tests/extension/decimal/array.py b/pandas/tests/extension/decimal/array.py index 387942234e6fd..f324cc2e0f345 100644 --- a/pandas/tests/extension/decimal/array.py +++ b/pandas/tests/extension/decimal/array.py @@ -138,5 +138,9 @@ def _concat_same_type(cls, to_concat): return cls(np.concatenate([x._data for x in to_concat])) +def to_decimal(values, context=None): + return DecimalArray([decimal.Decimal(x) for x in values], context=context) + + DecimalArray._add_arithmetic_ops() DecimalArray._add_comparison_ops() diff --git a/pandas/tests/extension/test_ops.py b/pandas/tests/extension/test_ops.py new file mode 100644 index 0000000000000..931194b474af4 --- /dev/null +++ b/pandas/tests/extension/test_ops.py @@ -0,0 +1,22 @@ +import pytest + +from pandas.tests.extension.decimal.array import to_decimal +import pandas.util.testing as tm + + +@pytest.mark.parametrize("reverse, expected_div, expected_mod", [ + (False, [0, 1, 1, 2], [1, 0, 1, 0]), + (True, [2, 1, 0, 0], [0, 0, 2, 2]), +]) +def test_divmod(reverse, expected_div, expected_mod): + # https://github.com/pandas-dev/pandas/issues/22930 + arr = to_decimal([1, 2, 3, 4]) + if reverse: + div, mod = divmod(2, arr) + else: + div, mod = divmod(arr, 2) + expected_div = to_decimal(expected_div) + expected_mod = to_decimal(expected_mod) + + tm.assert_extension_array_equal(div, expected_div) + tm.assert_extension_array_equal(mod, expected_mod) From c92a4a899b8d5e5e6a0479f390a604dc9f624f89 Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Mon, 1 Oct 2018 16:56:15 -0500 Subject: [PATCH 02/12] Update old test --- pandas/tests/extension/decimal/test_decimal.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/pandas/tests/extension/decimal/test_decimal.py b/pandas/tests/extension/decimal/test_decimal.py index 03fdd25826b79..420cd0b2f4159 100644 --- a/pandas/tests/extension/decimal/test_decimal.py +++ b/pandas/tests/extension/decimal/test_decimal.py @@ -252,9 +252,14 @@ def test_arith_series_with_array(self, data, all_arithmetic_operators): context.traps[decimal.DivisionByZero] = divbyzerotrap context.traps[decimal.InvalidOperation] = invalidoptrap - @pytest.mark.skip(reason="divmod not appropriate for decimal") def test_divmod(self, data): - pass + s = pd.Series(data, name='name') + a, b = divmod(s, 2) + ea, eb = zip(*(divmod(x, 2) for x in s)) + ea = pd.Series(ea, name=s.name, dtype=s.dtype) + eb = pd.Series(eb, name=s.name, dtype=s.dtype) + tm.assert_series_equal(a, ea) + tm.assert_series_equal(b, eb) def test_error(self): pass From 0671e7d67df8b0aa258fd864ef5f3169fe0ffc55 Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Tue, 2 Oct 2018 11:10:42 -0500 Subject: [PATCH 03/12] Fixup --- .../tests/extension/decimal/test_decimal.py | 19 +++++++++++++++- pandas/tests/extension/test_ops.py | 22 ------------------- 2 files changed, 18 insertions(+), 23 deletions(-) delete mode 100644 pandas/tests/extension/test_ops.py diff --git a/pandas/tests/extension/decimal/test_decimal.py b/pandas/tests/extension/decimal/test_decimal.py index 45a421dae0272..036e2d97b4d0f 100644 --- a/pandas/tests/extension/decimal/test_decimal.py +++ b/pandas/tests/extension/decimal/test_decimal.py @@ -8,7 +8,7 @@ from pandas.tests.extension import base -from .array import DecimalDtype, DecimalArray +from .array import DecimalDtype, DecimalArray, to_decimal def make_data(): @@ -244,6 +244,23 @@ def test_arith_series_with_array(self, data, all_arithmetic_operators): context.traps[decimal.DivisionByZero] = divbyzerotrap context.traps[decimal.InvalidOperation] = invalidoptrap + @pytest.mark.parametrize("reverse, expected_div, expected_mod", [ + (False, [0, 1, 1, 2], [1, 0, 1, 0]), + (True, [2, 1, 0, 0], [0, 0, 2, 2]), + ]) + def test_divmod_array(self, reverse, expected_div, expected_mod): + # https://github.com/pandas-dev/pandas/issues/22930 + arr = to_decimal([1, 2, 3, 4]) + if reverse: + div, mod = divmod(2, arr) + else: + div, mod = divmod(arr, 2) + expected_div = to_decimal(expected_div) + expected_mod = to_decimal(expected_mod) + + tm.assert_extension_array_equal(div, expected_div) + tm.assert_extension_array_equal(mod, expected_mod) + def test_divmod(self, data): s = pd.Series(data, name='name') a, b = divmod(s, 2) diff --git a/pandas/tests/extension/test_ops.py b/pandas/tests/extension/test_ops.py deleted file mode 100644 index 931194b474af4..0000000000000 --- a/pandas/tests/extension/test_ops.py +++ /dev/null @@ -1,22 +0,0 @@ -import pytest - -from pandas.tests.extension.decimal.array import to_decimal -import pandas.util.testing as tm - - -@pytest.mark.parametrize("reverse, expected_div, expected_mod", [ - (False, [0, 1, 1, 2], [1, 0, 1, 0]), - (True, [2, 1, 0, 0], [0, 0, 2, 2]), -]) -def test_divmod(reverse, expected_div, expected_mod): - # https://github.com/pandas-dev/pandas/issues/22930 - arr = to_decimal([1, 2, 3, 4]) - if reverse: - div, mod = divmod(2, arr) - else: - div, mod = divmod(arr, 2) - expected_div = to_decimal(expected_div) - expected_mod = to_decimal(expected_mod) - - tm.assert_extension_array_equal(div, expected_div) - tm.assert_extension_array_equal(mod, expected_mod) From 7ef697cffdcd2f8d701de3cdfd2e6897358effbf Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Wed, 3 Oct 2018 13:14:52 -0500 Subject: [PATCH 04/12] Use super --- pandas/tests/extension/decimal/test_decimal.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/pandas/tests/extension/decimal/test_decimal.py b/pandas/tests/extension/decimal/test_decimal.py index 036e2d97b4d0f..928ee83df8dc9 100644 --- a/pandas/tests/extension/decimal/test_decimal.py +++ b/pandas/tests/extension/decimal/test_decimal.py @@ -261,14 +261,11 @@ def test_divmod_array(self, reverse, expected_div, expected_mod): tm.assert_extension_array_equal(div, expected_div) tm.assert_extension_array_equal(mod, expected_mod) - def test_divmod(self, data): - s = pd.Series(data, name='name') - a, b = divmod(s, 2) - ea, eb = zip(*(divmod(x, 2) for x in s)) - ea = pd.Series(ea, name=s.name, dtype=s.dtype) - eb = pd.Series(eb, name=s.name, dtype=s.dtype) - tm.assert_series_equal(a, ea) - tm.assert_series_equal(b, eb) + def _check_divmod_op(self, s, op, other, exc=NotImplementedError): + # We implement divmod + super(TestArithmeticOps, self)._check_divmod_op( + s, op, other, exc=None + ) def test_error(self): pass From c9fe5d318d7077f99413532cdaf392ae3ea9cd2c Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Wed, 3 Oct 2018 13:21:33 -0500 Subject: [PATCH 05/12] make strict --- pandas/tests/extension/base/ops.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pandas/tests/extension/base/ops.py b/pandas/tests/extension/base/ops.py index ee4a92146128b..78b8ea0566dd0 100644 --- a/pandas/tests/extension/base/ops.py +++ b/pandas/tests/extension/base/ops.py @@ -58,7 +58,8 @@ def test_arith_series_with_scalar(self, data, all_arithmetic_operators): s = pd.Series(data) self.check_opname(s, op_name, s.iloc[0], exc=TypeError) - @pytest.mark.xfail(run=False, reason="_reduce needs implementation") + @pytest.mark.xfail(run=False, reason="_reduce needs implementation", + strict=True) def test_arith_frame_with_scalar(self, data, all_arithmetic_operators): # frame & scalar op_name = all_arithmetic_operators From 2247461ec0b1017db320cb8581337cba0b5c6679 Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Wed, 3 Oct 2018 13:29:29 -0500 Subject: [PATCH 06/12] Test op(Series[EA], EA]) --- pandas/tests/extension/base/ops.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pandas/tests/extension/base/ops.py b/pandas/tests/extension/base/ops.py index 78b8ea0566dd0..36696bc292162 100644 --- a/pandas/tests/extension/base/ops.py +++ b/pandas/tests/extension/base/ops.py @@ -78,6 +78,10 @@ def test_divmod(self, data): self._check_divmod_op(s, divmod, 1, exc=TypeError) self._check_divmod_op(1, ops.rdivmod, s, exc=TypeError) + def test_divmod_series_array(self, data): + s = pd.Series(data) + self._check_divmod_op(s, divmod, data) + def test_add_series_with_extension_array(self, data): s = pd.Series(data) result = s + data From a0cd5e79eb06ac71cf2f510b1a2122bc2b21fcf0 Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Wed, 3 Oct 2018 14:25:38 -0500 Subject: [PATCH 07/12] TypeError for Series --- pandas/tests/extension/test_categorical.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pandas/tests/extension/test_categorical.py b/pandas/tests/extension/test_categorical.py index c588552572aed..924f01077abd1 100644 --- a/pandas/tests/extension/test_categorical.py +++ b/pandas/tests/extension/test_categorical.py @@ -208,6 +208,11 @@ def test_add_series_with_extension_array(self, data): with tm.assert_raises_regex(TypeError, "cannot perform"): ser + data + def _check_divmod_op(self, s, op, other, exc=NotImplementedError): + return super(TestArithmeticOps, self)._check_divmod_op( + s, op, other, exc=TypeError + ) + class TestComparisonOps(base.BaseComparisonOpsTests): From 11a0d938cdaf7482546691519577b5dd28f69aac Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Wed, 3 Oct 2018 14:26:34 -0500 Subject: [PATCH 08/12] typerror --- pandas/tests/extension/json/test_json.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pandas/tests/extension/json/test_json.py b/pandas/tests/extension/json/test_json.py index 93f10b7fbfc23..e6dcbe30a949c 100644 --- a/pandas/tests/extension/json/test_json.py +++ b/pandas/tests/extension/json/test_json.py @@ -266,6 +266,11 @@ def test_add_series_with_extension_array(self, data): with tm.assert_raises_regex(TypeError, "unsupported"): ser + data + def _check_divmod_op(self, s, op, other, exc=NotImplementedError): + return super(TestArithmeticOps, self)._check_divmod_op( + s, op, other, exc=TypeError + ) + class TestComparisonOps(BaseJSON, base.BaseComparisonOpsTests): pass From 4e9b7f0a6ceec0275e22f5f1edac1daeb41f5033 Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Thu, 4 Oct 2018 08:18:40 -0500 Subject: [PATCH 09/12] doc update --- doc/source/extending.rst | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/doc/source/extending.rst b/doc/source/extending.rst index da249cb3592f4..ab940384594bc 100644 --- a/doc/source/extending.rst +++ b/doc/source/extending.rst @@ -167,11 +167,11 @@ your ``MyExtensionArray`` class, as follows: element one-by-one, this might not be as performant as implementing your own version of the associated operators directly on the ``ExtensionArray``. -This implementation will try to reconstruct a new ``ExtensionArray`` with the -result of the element-wise operation. Whether or not that succeeds depends on -whether the operation returns a result that's valid for the ``ExtensionArray``. -If an ``ExtensionArray`` cannot be reconstructed, a list containing the scalars -returned instead. +For arithmetic operations, this implementation will try to reconstruct a new +``ExtensionArray`` with the result of the element-wise operation. Whether +or not that succeeds depends on whether the operation returns a result +that's valid for the ``ExtensionArray``. If an ``ExtensionArray`` cannot +be reconstructed, an ndarray containing the scalars returned instead. .. _extending.extension.testing: From 95d5cbfe4eaf53ed60e84a938723062a14d2d625 Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Thu, 4 Oct 2018 08:22:17 -0500 Subject: [PATCH 10/12] typo, import --- pandas/core/arrays/base.py | 2 +- pandas/tests/extension/decimal/test_decimal.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pandas/core/arrays/base.py b/pandas/core/arrays/base.py index 29d01d6702796..efe587c6aaaad 100644 --- a/pandas/core/arrays/base.py +++ b/pandas/core/arrays/base.py @@ -787,7 +787,7 @@ def _maybe_convert(arr): # We catch all regular exceptions here, and fall back # to an ndarray. try: - res = self._from_sequnce(arr) + res = self._from_sequence(arr) except Exception: res = np.asarray(arr) else: diff --git a/pandas/tests/extension/decimal/test_decimal.py b/pandas/tests/extension/decimal/test_decimal.py index d07ac61b00ae9..a33cc6c4ab6cb 100644 --- a/pandas/tests/extension/decimal/test_decimal.py +++ b/pandas/tests/extension/decimal/test_decimal.py @@ -8,7 +8,7 @@ from pandas.tests.extension import base -from .array import DecimalDtype, DecimalArray, make_data +from .array import DecimalDtype, DecimalArray, make_data, to_decimal @pytest.fixture From c9d6e89a1f401e4f47b384b72030873cc4cc2f2b Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Thu, 4 Oct 2018 08:34:22 -0500 Subject: [PATCH 11/12] xpass -> skip --- pandas/tests/extension/decimal/test_decimal.py | 2 +- pandas/tests/extension/json/test_json.py | 12 ++++-------- pandas/tests/extension/test_categorical.py | 4 ++-- 3 files changed, 7 insertions(+), 11 deletions(-) diff --git a/pandas/tests/extension/decimal/test_decimal.py b/pandas/tests/extension/decimal/test_decimal.py index a33cc6c4ab6cb..317170e8db1e1 100644 --- a/pandas/tests/extension/decimal/test_decimal.py +++ b/pandas/tests/extension/decimal/test_decimal.py @@ -102,7 +102,7 @@ class TestInterface(BaseDecimal, base.BaseInterfaceTests): class TestConstructors(BaseDecimal, base.BaseConstructorsTests): - @pytest.mark.xfail(reason="not implemented constructor from dtype") + @pytest.mark.skip(reason="not implemented constructor from dtype") def test_from_dtype(self, data): # construct from our dtype & string dtype pass diff --git a/pandas/tests/extension/json/test_json.py b/pandas/tests/extension/json/test_json.py index e503e54db64c5..115afdcc99f2b 100644 --- a/pandas/tests/extension/json/test_json.py +++ b/pandas/tests/extension/json/test_json.py @@ -131,8 +131,7 @@ def test_custom_asserts(self): class TestConstructors(BaseJSON, base.BaseConstructorsTests): - # TODO: Should this be pytest.mark.skip? - @pytest.mark.xfail(reason="not implemented constructor from dtype") + @pytest.mark.skip(reason="not implemented constructor from dtype") def test_from_dtype(self, data): # construct from our dtype & string dtype pass @@ -147,13 +146,11 @@ class TestGetitem(BaseJSON, base.BaseGetitemTests): class TestMissing(BaseJSON, base.BaseMissingTests): - # TODO: Should this be pytest.mark.skip? - @pytest.mark.xfail(reason="Setting a dict as a scalar") + @pytest.mark.skip(reason="Setting a dict as a scalar") def test_fillna_series(self): """We treat dictionaries as a mapping in fillna, not a scalar.""" - # TODO: Should this be pytest.mark.skip? - @pytest.mark.xfail(reason="Setting a dict as a scalar") + @pytest.mark.skip(reason="Setting a dict as a scalar") def test_fillna_frame(self): """We treat dictionaries as a mapping in fillna, not a scalar.""" @@ -204,8 +201,7 @@ def test_combine_add(self, data_repeated): class TestCasting(BaseJSON, base.BaseCastingTests): - # TODO: Should this be pytest.mark.skip? - @pytest.mark.xfail(reason="failing on np.array(self, dtype=str)") + @pytest.mark.skip(reason="failing on np.array(self, dtype=str)") def test_astype_str(self): """This currently fails in NumPy on np.array(self, dtype=str) with diff --git a/pandas/tests/extension/test_categorical.py b/pandas/tests/extension/test_categorical.py index 924f01077abd1..f118279c4b915 100644 --- a/pandas/tests/extension/test_categorical.py +++ b/pandas/tests/extension/test_categorical.py @@ -140,11 +140,11 @@ def test_take_series(self): def test_reindex_non_na_fill_value(self): pass - @pytest.mark.xfail(reason="Categorical.take buggy") + @pytest.mark.skip(reason="Categorical.take buggy") def test_take_empty(self): pass - @pytest.mark.xfail(reason="test not written correctly for categorical") + @pytest.mark.skip(reason="test not written correctly for categorical") def test_reindex(self): pass From ec814db9acea380be0b9e82474e7365bf2a20536 Mon Sep 17 00:00:00 2001 From: Tom Augspurger Date: Fri, 5 Oct 2018 06:51:09 -0500 Subject: [PATCH 12/12] Move --- .../tests/extension/decimal/test_decimal.py | 35 ++++++++++--------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/pandas/tests/extension/decimal/test_decimal.py b/pandas/tests/extension/decimal/test_decimal.py index 317170e8db1e1..6488c7724229b 100644 --- a/pandas/tests/extension/decimal/test_decimal.py +++ b/pandas/tests/extension/decimal/test_decimal.py @@ -240,23 +240,6 @@ def test_arith_series_with_array(self, data, all_arithmetic_operators): context.traps[decimal.DivisionByZero] = divbyzerotrap context.traps[decimal.InvalidOperation] = invalidoptrap - @pytest.mark.parametrize("reverse, expected_div, expected_mod", [ - (False, [0, 1, 1, 2], [1, 0, 1, 0]), - (True, [2, 1, 0, 0], [0, 0, 2, 2]), - ]) - def test_divmod_array(self, reverse, expected_div, expected_mod): - # https://github.com/pandas-dev/pandas/issues/22930 - arr = to_decimal([1, 2, 3, 4]) - if reverse: - div, mod = divmod(2, arr) - else: - div, mod = divmod(arr, 2) - expected_div = to_decimal(expected_div) - expected_mod = to_decimal(expected_mod) - - tm.assert_extension_array_equal(div, expected_div) - tm.assert_extension_array_equal(mod, expected_mod) - def _check_divmod_op(self, s, op, other, exc=NotImplementedError): # We implement divmod super(TestArithmeticOps, self)._check_divmod_op( @@ -334,3 +317,21 @@ def test_scalar_ops_from_sequence_raises(class_): expected = np.array([decimal.Decimal("2.0"), decimal.Decimal("4.0")], dtype="object") tm.assert_numpy_array_equal(result, expected) + + +@pytest.mark.parametrize("reverse, expected_div, expected_mod", [ + (False, [0, 1, 1, 2], [1, 0, 1, 0]), + (True, [2, 1, 0, 0], [0, 0, 2, 2]), +]) +def test_divmod_array(reverse, expected_div, expected_mod): + # https://github.com/pandas-dev/pandas/issues/22930 + arr = to_decimal([1, 2, 3, 4]) + if reverse: + div, mod = divmod(2, arr) + else: + div, mod = divmod(arr, 2) + expected_div = to_decimal(expected_div) + expected_mod = to_decimal(expected_mod) + + tm.assert_extension_array_equal(div, expected_div) + tm.assert_extension_array_equal(mod, expected_mod)