From b69c2cb5e01406956ba356349035e47b8b01f463 Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Sat, 5 Jan 2019 08:06:17 -0800 Subject: [PATCH 1/2] Make DTA/TDA/PA return NotImplemented on comparisons --- pandas/core/arrays/datetimes.py | 8 ++++---- pandas/core/arrays/period.py | 17 ++++++----------- pandas/core/arrays/timedeltas.py | 3 +++ pandas/core/indexes/datetimelike.py | 9 +++++++-- pandas/tests/arithmetic/test_period.py | 10 ++++++++-- 5 files changed, 28 insertions(+), 19 deletions(-) diff --git a/pandas/core/arrays/datetimes.py b/pandas/core/arrays/datetimes.py index e6fbc6d1f4b15..b858dc0a8d54a 100644 --- a/pandas/core/arrays/datetimes.py +++ b/pandas/core/arrays/datetimes.py @@ -19,7 +19,8 @@ is_extension_type, is_float_dtype, is_int64_dtype, is_object_dtype, is_period_dtype, is_string_dtype, is_timedelta64_dtype, pandas_dtype) from pandas.core.dtypes.dtypes import DatetimeTZDtype -from pandas.core.dtypes.generic import ABCIndexClass, ABCPandasArray, ABCSeries +from pandas.core.dtypes.generic import ( + ABCDataFrame, ABCIndexClass, ABCPandasArray, ABCSeries) from pandas.core.dtypes.missing import isna from pandas.core import ops @@ -96,9 +97,8 @@ def _dt_array_cmp(cls, op): nat_result = True if opname == '__ne__' else False def wrapper(self, other): - # TODO: return NotImplemented for Series / Index and let pandas unbox - # Right now, returning NotImplemented for Index fails because we - # go into the index implementation, which may be a bug? + if isinstance(other, (ABCDataFrame, ABCSeries, ABCIndexClass)): + return NotImplemented other = lib.item_from_zerodim(other) diff --git a/pandas/core/arrays/period.py b/pandas/core/arrays/period.py index 34bb03b249c21..513bd7223e880 100644 --- a/pandas/core/arrays/period.py +++ b/pandas/core/arrays/period.py @@ -17,7 +17,8 @@ _TD_DTYPE, ensure_object, is_datetime64_dtype, is_float_dtype, is_list_like, is_period_dtype, pandas_dtype) from pandas.core.dtypes.dtypes import PeriodDtype -from pandas.core.dtypes.generic import ABCIndexClass, ABCPeriodIndex, ABCSeries +from pandas.core.dtypes.generic import ( + ABCDataFrame, ABCIndexClass, ABCPeriodIndex, ABCSeries) from pandas.core.dtypes.missing import isna, notna import pandas.core.algorithms as algos @@ -48,17 +49,13 @@ def _period_array_cmp(cls, op): def wrapper(self, other): op = getattr(self.asi8, opname) - # We want to eventually defer to the Series or PeriodIndex (which will - # return here with an unboxed PeriodArray). But before we do that, - # we do a bit of validation on type (Period) and freq, so that our - # error messages are sensible + + if isinstance(other, (ABCDataFrame, ABCSeries, ABCIndexClass)): + return NotImplemented + if is_list_like(other) and len(other) != len(self): raise ValueError("Lengths must match") - not_implemented = isinstance(other, (ABCSeries, ABCIndexClass)) - if not_implemented: - other = other._values - if isinstance(other, Period): self._check_compatible_with(other) @@ -66,8 +63,6 @@ def wrapper(self, other): elif isinstance(other, cls): self._check_compatible_with(other) - if not_implemented: - return NotImplemented result = op(other.asi8) mask = self._isnan | other._isnan diff --git a/pandas/core/arrays/timedeltas.py b/pandas/core/arrays/timedeltas.py index ab9986b5bff69..624305ec4303d 100644 --- a/pandas/core/arrays/timedeltas.py +++ b/pandas/core/arrays/timedeltas.py @@ -64,6 +64,9 @@ def _td_array_cmp(cls, op): nat_result = True if opname == '__ne__' else False def wrapper(self, other): + if isinstance(other, (ABCDataFrame, ABCSeries, ABCIndexClass)): + return NotImplemented + if _is_convertible_to_td(other) or other is NaT: try: other = Timedelta(other) diff --git a/pandas/core/indexes/datetimelike.py b/pandas/core/indexes/datetimelike.py index 5a8809f754385..d50609bf48ef0 100644 --- a/pandas/core/indexes/datetimelike.py +++ b/pandas/core/indexes/datetimelike.py @@ -109,7 +109,8 @@ def _create_comparison_method(cls, op): Create a comparison method that dispatches to ``cls.values``. """ def wrapper(self, other): - result = op(self._data, maybe_unwrap_index(other)) + other = maybe_unwrap_index(other, unwrap_series=True) + result = op(self._data, other) return result wrapper.__doc__ = op.__doc__ @@ -655,7 +656,7 @@ def wrap_arithmetic_op(self, other, result): return result -def maybe_unwrap_index(obj): +def maybe_unwrap_index(obj, unwrap_series=False): """ If operating against another Index object, we need to unwrap the underlying data before deferring to the DatetimeArray/TimedeltaArray/PeriodArray @@ -664,6 +665,8 @@ def maybe_unwrap_index(obj): Parameters ---------- obj : object + unwrap_series : bool + Whether to also unwrap Series objects. Returns ------- @@ -671,6 +674,8 @@ def maybe_unwrap_index(obj): """ if isinstance(obj, ABCIndexClass): return obj._data + elif isinstance(obj, ABCSeries) and unwrap_series: + obj = obj._values return obj diff --git a/pandas/tests/arithmetic/test_period.py b/pandas/tests/arithmetic/test_period.py index cdacd4b42d683..92f209b94f00d 100644 --- a/pandas/tests/arithmetic/test_period.py +++ b/pandas/tests/arithmetic/test_period.py @@ -152,7 +152,10 @@ def test_parr_cmp_pi_mismatched_freq_raises(self, freq, box_with_array): # TODO: Could parametrize over boxes for idx? idx = PeriodIndex(['2011', '2012', '2013', '2014'], freq='A') - with pytest.raises(IncompatibleFrequency, match=msg): + rev_msg = (r'Input has different freq=(M|2M|3M) from ' + r'PeriodArray\(freq=A-DEC\)') + idx_msg = rev_msg if box_with_array is tm.to_array else msg + with pytest.raises(IncompatibleFrequency, match=idx_msg): base <= idx # Different frequency @@ -164,7 +167,10 @@ def test_parr_cmp_pi_mismatched_freq_raises(self, freq, box_with_array): Period('2011', freq='4M') >= base idx = PeriodIndex(['2011', '2012', '2013', '2014'], freq='4M') - with pytest.raises(IncompatibleFrequency, match=msg): + rev_msg = (r'Input has different freq=(M|2M|3M) from ' + r'PeriodArray\(freq=4M\)') + idx_msg = rev_msg if box_with_array is tm.to_array else msg + with pytest.raises(IncompatibleFrequency, match=idx_msg): base <= idx @pytest.mark.parametrize('freq', ['M', '2M', '3M']) From 33cdf670eecf362d01d5e6fc75e69a731be6661b Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Sat, 5 Jan 2019 12:55:58 -0800 Subject: [PATCH 2/2] de-func --- pandas/core/indexes/datetimelike.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/pandas/core/indexes/datetimelike.py b/pandas/core/indexes/datetimelike.py index d50609bf48ef0..aa7332472fc07 100644 --- a/pandas/core/indexes/datetimelike.py +++ b/pandas/core/indexes/datetimelike.py @@ -109,8 +109,12 @@ def _create_comparison_method(cls, op): Create a comparison method that dispatches to ``cls.values``. """ def wrapper(self, other): - other = maybe_unwrap_index(other, unwrap_series=True) - result = op(self._data, other) + if isinstance(other, ABCSeries): + # the arrays defer to Series for comparison ops but the indexes + # don't, so we have to unwrap here. + other = other._values + + result = op(self._data, maybe_unwrap_index(other)) return result wrapper.__doc__ = op.__doc__ @@ -656,7 +660,7 @@ def wrap_arithmetic_op(self, other, result): return result -def maybe_unwrap_index(obj, unwrap_series=False): +def maybe_unwrap_index(obj): """ If operating against another Index object, we need to unwrap the underlying data before deferring to the DatetimeArray/TimedeltaArray/PeriodArray @@ -665,8 +669,6 @@ def maybe_unwrap_index(obj, unwrap_series=False): Parameters ---------- obj : object - unwrap_series : bool - Whether to also unwrap Series objects. Returns ------- @@ -674,8 +676,6 @@ def maybe_unwrap_index(obj, unwrap_series=False): """ if isinstance(obj, ABCIndexClass): return obj._data - elif isinstance(obj, ABCSeries) and unwrap_series: - obj = obj._values return obj