From e7d1f4a0832089f030bdbf929a58a03b3a09c44d Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Mon, 11 Dec 2017 17:50:39 -0800 Subject: [PATCH 1/5] handle comparison of Timedelta with Tick --- pandas/_libs/tslibs/timedeltas.pyx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/pandas/_libs/tslibs/timedeltas.pyx b/pandas/_libs/tslibs/timedeltas.pyx index b37e5dc620260..6b0dbedbbdfc6 100644 --- a/pandas/_libs/tslibs/timedeltas.pyx +++ b/pandas/_libs/tslibs/timedeltas.pyx @@ -567,7 +567,13 @@ cdef class _Timedelta(timedelta): return PyObject_RichCompare(np.array([self]), other, op) return PyObject_RichCompare(other, self, reverse_ops[op]) else: - if op == Py_EQ: + if (getattr(other, "_typ", "") == "dateoffset" and + hasattr(other, "delta")): + # offsets.Tick; we catch this fairly late as it is a + # relatively infrequent case + ots = other.delta + return cmp_scalar(self.value, ots.value, op) + elif op == Py_EQ: return False elif op == Py_NE: return True From 857db79b5870c5230e44557856a3f5f24c76dbbc Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Mon, 11 Dec 2017 18:07:15 -0800 Subject: [PATCH 2/5] implement comparison methods more thoroughly --- pandas/tests/tseries/offsets/test_offsets.py | 48 ++++++++++ pandas/tests/tseries/offsets/test_ticks.py | 21 ++++- pandas/tseries/offsets.py | 93 +++++++++++++------- 3 files changed, 130 insertions(+), 32 deletions(-) diff --git a/pandas/tests/tseries/offsets/test_offsets.py b/pandas/tests/tseries/offsets/test_offsets.py index 84e811301ab4b..55f46a6793fc6 100644 --- a/pandas/tests/tseries/offsets/test_offsets.py +++ b/pandas/tests/tseries/offsets/test_offsets.py @@ -3187,3 +3187,51 @@ def test_require_integers(offset_types): cls = offset_types with pytest.raises(ValueError): cls(n=1.5) + + +def test_comparisons(offset_types): + cls = offset_types + + if cls is WeekOfMonth: + # TODO: The default values for week and weekday should be non-raising + off = cls(n=1, week=1, weekday=2) + elif cls is LastWeekOfMonth: + # TODO: The default value for weekday should be non-raising + off = cls(n=1, weekday=4) + else: + off = cls(n=1) + + if cls is Week: + assert off < timedelta(days=8) + assert off > timedelta(days=6) + assert off <= Day(n=7) + elif issubclass(cls, offsets.Tick): + pass + else: + with pytest.raises(TypeError): + off < timedelta(days=8) + with pytest.raises(TypeError): + off > timedelta(days=6) + with pytest.raises(TypeError): + off <= Day(n=7) + + +def test_week_comparison(): + # Only Week with weekday == None is special + off = Week(weekday=3) + with pytest.raises(TypeError): + off < timedelta(days=8) + with pytest.raises(TypeError): + off > timedelta(days=6) + with pytest.raises(TypeError): + off <= Day(n=7) + + +def test_comparison_names(offset_types): + cls = offset_types + assert cls.__eq__.__name__ == '__eq__' + assert cls.__ne__.__name__ == '__ne__' + assert cls.__lt__.__name__ == '__lt__' + assert cls.__le__.__name__ == '__le__' + assert cls.__gt__.__name__ == '__gt__' + assert cls.__ge__.__name__ == '__ge__' diff --git a/pandas/tests/tseries/offsets/test_ticks.py b/pandas/tests/tseries/offsets/test_ticks.py index 24033d4ff6cbd..d82062c96408b 100644 --- a/pandas/tests/tseries/offsets/test_ticks.py +++ b/pandas/tests/tseries/offsets/test_ticks.py @@ -9,7 +9,8 @@ from pandas import Timedelta, Timestamp from pandas.tseries import offsets -from pandas.tseries.offsets import Hour, Minute, Second, Milli, Micro, Nano +from pandas.tseries.offsets import (Hour, Minute, Second, Milli, Micro, Nano, + Week) from .common import assert_offset_equal @@ -35,6 +36,24 @@ def test_delta_to_tick(): assert (tick == offsets.Day(3)) +@pytest.mark.parametrize('cls', tick_classes) +def test_tick_comparisons(cls): + off = cls(n=2) + with pytest.raises(TypeError): + off < 3 + + # Unfortunately there is no good way to make the reverse inequality work + assert off > timedelta(-1) + assert off >= timedelta(-1) + assert off < off._inc * 3 # Timedelta object + assert off <= off._inc * 3 # Timedelta object + assert off == off.delta + assert off.delta == off + assert off != -1 * off + + assert off < Week() + + # --------------------------------------------------------------------- diff --git a/pandas/tseries/offsets.py b/pandas/tseries/offsets.py index dd5f01a36a43e..227af03c01cb5 100644 --- a/pandas/tseries/offsets.py +++ b/pandas/tseries/offsets.py @@ -112,6 +112,24 @@ def wrapper(self, other): return wrapper +def _make_cmp_func(op): + assert op not in [operator.eq, operator.ne] + # __eq__ and __ne__ have slightly different behavior, returning + # False and True, respectively, instead of raising + + def cmp_func(self, other): + if type(self) == Week and self.weekday is None: + # Week without weekday behaves like a Tick + tick = Day(n=7 * self.n, normalize=self.normalize) + return op(tick, other) + else: + raise TypeError('Cannot compare type {self} with {other}' + .format(self=self.__class__.__name__, + other=other.__class__.__name__)) + + cmp_func.__name__ = '__{name}__'.format(name=op.__name__) + return cmp_func + # --------------------------------------------------------------------- # DateOffset @@ -299,6 +317,11 @@ def _repr_attrs(self): def name(self): return self.rule_code + __lt__ = _make_cmp_func(operator.lt) + __le__ = _make_cmp_func(operator.le) + __ge__ = _make_cmp_func(operator.ge) + __gt__ = _make_cmp_func(operator.gt) + def __eq__(self, other): if other is None: return False @@ -320,9 +343,7 @@ def __hash__(self): return hash(self._params()) def __add__(self, other): - if isinstance(other, (ABCDatetimeIndex, ABCSeries)): - return other + self - elif isinstance(other, ABCPeriod): + if isinstance(other, (ABCDatetimeIndex, ABCSeries, ABCPeriod)): return other + self try: return self.apply(other) @@ -2145,8 +2166,41 @@ def onOffset(self, dt): def _tick_comp(op): def f(self, other): - return op(self.delta, other.delta) + if isinstance(other, Tick): + # Note we cannot just try/except other.delta because Tick.delta + # returns a Timedelta while Timedelta.delta returns an int + other_delta = other.delta + elif isinstance(other, (timedelta, np.timedelta64)): + other_delta = other + elif isinstance(other, Week) and other.weekday is None: + other_delta = timedelta(weeks=other.n) + elif isinstance(other, compat.string_types): + from pandas.tseries.frequencies import to_offset + other = to_offset(other) + if isinstance(other, DateOffset): + return f(self, other) + else: + if op == operator.eq: + return False + elif op == operator.ne: + return True + raise TypeError('Cannot compare type {self} and {other}' + .format(self=self.__class__.__name__, + other=other.__class__.__name__)) + elif op == operator.eq: + # TODO: Consider changing this older behavior for + # __eq__ and __ne__to match other comparisons + return False + elif op == operator.ne: + return True + else: + raise TypeError('Cannot compare type {self} and {other}' + .format(self=self.__class__.__name__, + other=other.__class__.__name__)) + + return op(self.delta, other_delta) + f.__name__ = '__{name}__'.format(name=op.__name__) return f @@ -2183,34 +2237,11 @@ def __add__(self, other): raise OverflowError("the add operation between {self} and {other} " "will overflow".format(self=self, other=other)) - def __eq__(self, other): - if isinstance(other, compat.string_types): - from pandas.tseries.frequencies import to_offset - - other = to_offset(other) - - if isinstance(other, Tick): - return self.delta == other.delta - else: - # TODO: Are there cases where this should raise TypeError? - return False - - # This is identical to DateOffset.__hash__, but has to be redefined here - # for Python 3, because we've redefined __eq__. def __hash__(self): - return hash(self._params()) - - def __ne__(self, other): - if isinstance(other, compat.string_types): - from pandas.tseries.frequencies import to_offset - - other = to_offset(other) - - if isinstance(other, Tick): - return self.delta != other.delta - else: - # TODO: Are there cases where this should raise TypeError? - return True + # This is identical to DateOffset.__hash__, but has to be redefined + # here for Python 3, because we've redefined __eq__. + tup = (str(self.__class__), ('n', self.n)) + return hash(tup) @property def delta(self): From e18168bb956aab8f4933ab5a5569d37119de4f61 Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Mon, 11 Dec 2017 18:19:07 -0800 Subject: [PATCH 3/5] tests for timedelta comparisons --- pandas/tests/scalar/test_timedelta.py | 15 +++++++++++++++ pandas/tests/tseries/offsets/test_offsets.py | 2 ++ 2 files changed, 17 insertions(+) diff --git a/pandas/tests/scalar/test_timedelta.py b/pandas/tests/scalar/test_timedelta.py index 001f6c1fdbef4..ba35b6d0d8632 100644 --- a/pandas/tests/scalar/test_timedelta.py +++ b/pandas/tests/scalar/test_timedelta.py @@ -9,9 +9,24 @@ from pandas.core.tools.timedeltas import _coerce_scalar_to_timedelta_type as ct from pandas import (Timedelta, TimedeltaIndex, timedelta_range, Series, to_timedelta, compat) +from pandas.tseries.frequencies import to_offset from pandas._libs.tslib import iNaT, NaT +class TestTimedeltaComparisons(object): + @pytest.mark.parametrize('freq', ['D', 'H', 'T', 's', 'ms', 'us', 'ns']) + def test_tick_comparison(self, freq): + offset = to_offset(freq) * 2 + delta = offset.delta + assert isinstance(delta, Timedelta) + assert delta < offset + assert delta <= offset + assert not delta == offset + assert delta != offset + assert not delta > offset + assert not delta >= offset + + class TestTimedeltaArithmetic(object): _multiprocess_can_split_ = True diff --git a/pandas/tests/tseries/offsets/test_offsets.py b/pandas/tests/tseries/offsets/test_offsets.py index 55f46a6793fc6..a25dcd1c431ee 100644 --- a/pandas/tests/tseries/offsets/test_offsets.py +++ b/pandas/tests/tseries/offsets/test_offsets.py @@ -3214,6 +3214,8 @@ def test_comparisons(offset_types): off > timedelta(days=6) with pytest.raises(TypeError): off <= Day(n=7) + with pytest.raises(TypeError): + off < DateOffset(month=7) def test_week_comparison(): From 6a97205a2cd34873afe26e1d58113198d514da40 Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Tue, 12 Dec 2017 11:10:26 -0800 Subject: [PATCH 4/5] fix mis-specified test --- pandas/tests/scalar/test_timedelta.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/tests/scalar/test_timedelta.py b/pandas/tests/scalar/test_timedelta.py index ba35b6d0d8632..a9068302ad7fc 100644 --- a/pandas/tests/scalar/test_timedelta.py +++ b/pandas/tests/scalar/test_timedelta.py @@ -17,7 +17,7 @@ class TestTimedeltaComparisons(object): @pytest.mark.parametrize('freq', ['D', 'H', 'T', 's', 'ms', 'us', 'ns']) def test_tick_comparison(self, freq): offset = to_offset(freq) * 2 - delta = offset.delta + delta = offset._inc assert isinstance(delta, Timedelta) assert delta < offset assert delta <= offset From 343dc4cea936db2656ea521541ddb1429bcef8ed Mon Sep 17 00:00:00 2001 From: Brock Mendel Date: Thu, 21 Dec 2017 08:17:20 -0800 Subject: [PATCH 5/5] parametrize test --- pandas/tests/tseries/offsets/test_offsets.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/pandas/tests/tseries/offsets/test_offsets.py b/pandas/tests/tseries/offsets/test_offsets.py index a25dcd1c431ee..0371ebe64e04b 100644 --- a/pandas/tests/tseries/offsets/test_offsets.py +++ b/pandas/tests/tseries/offsets/test_offsets.py @@ -3229,11 +3229,10 @@ def test_week_comparison(): off <= Day(n=7) -def test_comparison_names(offset_types): +@pytest.mark.parametrize('opname', ['__eq__', '__ne__', + '__lt__', '__le__', + '__gt__', '__ge__']) +def test_comparison_names(offset_types, opname): cls = offset_types - assert cls.__eq__.__name__ == '__eq__' - assert cls.__ne__.__name__ == '__ne__' - assert cls.__lt__.__name__ == '__lt__' - assert cls.__le__.__name__ == '__le__' - assert cls.__gt__.__name__ == '__gt__' - assert cls.__ge__.__name__ == '__ge__' + method = getattr(cls, opname) + assert method.__name__ == opname