Skip to content

Commit 5dd4cae

Browse files
Sup3rGeojbrockmendel
authored andcommitted
Bugfix timedelta notimplemented eq (#21394)
* Return NotImplemented for unknown types in comparison. * Added tests and updated whatsnew. * Removed excess space LINT * TST/BUG Always return NotImplemented. Updated tests to take into account that python 2 does not raise TypeError for uncompatible types. * Line too long. * Fixed version_info call in test. * Moved changes to v0.24.0 * Skipped test based on PY2 * Updated test names. * Removed unused import * Fixed whatsnew placement and commented test.
1 parent 92fd46c commit 5dd4cae

File tree

3 files changed

+57
-20
lines changed

3 files changed

+57
-20
lines changed

doc/source/whatsnew/v0.24.0.txt

+2-1
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,7 @@ Other Enhancements
193193
- :func:`~DataFrame.to_csv`, :func:`~Series.to_csv`, :func:`~DataFrame.to_json`, and :func:`~Series.to_json` now support ``compression='infer'`` to infer compression based on filename extension (:issue:`15008`).
194194
The default compression for ``to_csv``, ``to_json``, and ``to_pickle`` methods has been updated to ``'infer'`` (:issue:`22004`).
195195
- :func:`to_timedelta` now supports iso-formated timedelta strings (:issue:`21877`)
196+
- Comparing :class:`Timedelta` with unknown types now return ``NotImplemented`` instead of ``False`` (:issue:`20829`)
196197
- :class:`Series` and :class:`DataFrame` now support :class:`Iterable` in constructor (:issue:`2193`)
197198
- :class:`DatetimeIndex` gained :attr:`DatetimeIndex.timetz` attribute. Returns local time with timezone information. (:issue:`21358`)
198199
- :meth:`round`, :meth:`ceil`, and meth:`floor` for :class:`DatetimeIndex` and :class:`Timestamp` now support an ``ambiguous`` argument for handling datetimes that are rounded to ambiguous times (:issue:`18946`)
@@ -1011,7 +1012,7 @@ Timedelta
10111012
- Bug in :class:`TimedeltaIndex` incorrectly allowing indexing with ``Timestamp`` object (:issue:`20464`)
10121013
- Fixed bug where subtracting :class:`Timedelta` from an object-dtyped array would raise ``TypeError`` (:issue:`21980`)
10131014
- Fixed bug in adding a :class:`DataFrame` with all-`timedelta64[ns]` dtypes to a :class:`DataFrame` with all-integer dtypes returning incorrect results instead of raising ``TypeError`` (:issue:`22696`)
1014-
-
1015+
10151016

10161017
Timezones
10171018
^^^^^^^^^

pandas/_libs/tslibs/timedeltas.pyx

+2-17
Original file line numberDiff line numberDiff line change
@@ -724,27 +724,12 @@ cdef class _Timedelta(timedelta):
724724
if is_timedelta64_object(other):
725725
other = Timedelta(other)
726726
else:
727-
if op == Py_EQ:
728-
return False
729-
elif op == Py_NE:
730-
return True
731-
732-
# only allow ==, != ops
733-
raise TypeError('Cannot compare type {cls} with '
734-
'type {other}'
735-
.format(cls=type(self).__name__,
736-
other=type(other).__name__))
727+
return NotImplemented
737728
if util.is_array(other):
738729
return PyObject_RichCompare(np.array([self]), other, op)
739730
return PyObject_RichCompare(other, self, reverse_ops[op])
740731
else:
741-
if op == Py_EQ:
742-
return False
743-
elif op == Py_NE:
744-
return True
745-
raise TypeError('Cannot compare type {cls} with type {other}'
746-
.format(cls=type(self).__name__,
747-
other=type(other).__name__))
732+
return NotImplemented
748733

749734
return cmp_scalar(self.value, ots.value, op)
750735

pandas/tests/scalar/timedelta/test_timedelta.py

+53-2
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,10 @@ def test_ops_error_str(self):
4242
with pytest.raises(TypeError):
4343
left + right
4444

45-
with pytest.raises(TypeError):
46-
left > right
45+
# GH 20829: python 2 comparison naturally does not raise TypeError
46+
if compat.PY3:
47+
with pytest.raises(TypeError):
48+
left > right
4749

4850
assert not left == right
4951
assert left != right
@@ -103,6 +105,55 @@ def test_compare_timedelta_ndarray(self):
103105
expected = np.array([False, False])
104106
tm.assert_numpy_array_equal(result, expected)
105107

108+
def test_compare_custom_object(self):
109+
"""Make sure non supported operations on Timedelta returns NonImplemented
110+
and yields to other operand (GH20829)."""
111+
class CustomClass(object):
112+
113+
def __init__(self, cmp_result=None):
114+
self.cmp_result = cmp_result
115+
116+
def generic_result(self):
117+
if self.cmp_result is None:
118+
return NotImplemented
119+
else:
120+
return self.cmp_result
121+
122+
def __eq__(self, other):
123+
return self.generic_result()
124+
125+
def __gt__(self, other):
126+
return self.generic_result()
127+
128+
t = Timedelta('1s')
129+
130+
assert not (t == "string")
131+
assert not (t == 1)
132+
assert not (t == CustomClass())
133+
assert not (t == CustomClass(cmp_result=False))
134+
135+
assert t < CustomClass(cmp_result=True)
136+
assert not (t < CustomClass(cmp_result=False))
137+
138+
assert t == CustomClass(cmp_result=True)
139+
140+
@pytest.mark.skipif(compat.PY2,
141+
reason="python 2 does not raise TypeError for \
142+
comparisons of different types")
143+
@pytest.mark.parametrize("val", [
144+
"string", 1])
145+
def test_compare_unknown_type(self, val):
146+
# GH20829
147+
t = Timedelta('1s')
148+
with pytest.raises(TypeError):
149+
t >= val
150+
with pytest.raises(TypeError):
151+
t > val
152+
with pytest.raises(TypeError):
153+
t <= val
154+
with pytest.raises(TypeError):
155+
t < val
156+
106157

107158
class TestTimedeltas(object):
108159

0 commit comments

Comments
 (0)