Skip to content

Commit

Permalink
Bugfix timedelta notimplemented eq (pandas-dev#21394)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
Sup3rGeo authored and tm9k1 committed Nov 19, 2018
1 parent 147e243 commit fce8cc2
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 20 deletions.
3 changes: 2 additions & 1 deletion doc/source/whatsnew/v0.24.0.txt
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ Other Enhancements
- :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`).
The default compression for ``to_csv``, ``to_json``, and ``to_pickle`` methods has been updated to ``'infer'`` (:issue:`22004`).
- :func:`to_timedelta` now supports iso-formated timedelta strings (:issue:`21877`)
- Comparing :class:`Timedelta` with unknown types now return ``NotImplemented`` instead of ``False`` (:issue:`20829`)
- :class:`Series` and :class:`DataFrame` now support :class:`Iterable` in constructor (:issue:`2193`)
- :class:`DatetimeIndex` gained :attr:`DatetimeIndex.timetz` attribute. Returns local time with timezone information. (:issue:`21358`)
- :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`)
Expand Down Expand Up @@ -1012,7 +1013,7 @@ Timedelta
- Bug in :class:`TimedeltaIndex` incorrectly allowing indexing with ``Timestamp`` object (:issue:`20464`)
- Fixed bug where subtracting :class:`Timedelta` from an object-dtyped array would raise ``TypeError`` (:issue:`21980`)
- 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`)
-


Timezones
^^^^^^^^^
Expand Down
19 changes: 2 additions & 17 deletions pandas/_libs/tslibs/timedeltas.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -724,27 +724,12 @@ cdef class _Timedelta(timedelta):
if is_timedelta64_object(other):
other = Timedelta(other)
else:
if op == Py_EQ:
return False
elif op == Py_NE:
return True

# only allow ==, != ops
raise TypeError('Cannot compare type {cls} with '
'type {other}'
.format(cls=type(self).__name__,
other=type(other).__name__))
return NotImplemented
if util.is_array(other):
return PyObject_RichCompare(np.array([self]), other, op)
return PyObject_RichCompare(other, self, reverse_ops[op])
else:
if op == Py_EQ:
return False
elif op == Py_NE:
return True
raise TypeError('Cannot compare type {cls} with type {other}'
.format(cls=type(self).__name__,
other=type(other).__name__))
return NotImplemented

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

Expand Down
55 changes: 53 additions & 2 deletions pandas/tests/scalar/timedelta/test_timedelta.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,10 @@ def test_ops_error_str(self):
with pytest.raises(TypeError):
left + right

with pytest.raises(TypeError):
left > right
# GH 20829: python 2 comparison naturally does not raise TypeError
if compat.PY3:
with pytest.raises(TypeError):
left > right

assert not left == right
assert left != right
Expand Down Expand Up @@ -103,6 +105,55 @@ def test_compare_timedelta_ndarray(self):
expected = np.array([False, False])
tm.assert_numpy_array_equal(result, expected)

def test_compare_custom_object(self):
"""Make sure non supported operations on Timedelta returns NonImplemented
and yields to other operand (GH20829)."""
class CustomClass(object):

def __init__(self, cmp_result=None):
self.cmp_result = cmp_result

def generic_result(self):
if self.cmp_result is None:
return NotImplemented
else:
return self.cmp_result

def __eq__(self, other):
return self.generic_result()

def __gt__(self, other):
return self.generic_result()

t = Timedelta('1s')

assert not (t == "string")
assert not (t == 1)
assert not (t == CustomClass())
assert not (t == CustomClass(cmp_result=False))

assert t < CustomClass(cmp_result=True)
assert not (t < CustomClass(cmp_result=False))

assert t == CustomClass(cmp_result=True)

@pytest.mark.skipif(compat.PY2,
reason="python 2 does not raise TypeError for \
comparisons of different types")
@pytest.mark.parametrize("val", [
"string", 1])
def test_compare_unknown_type(self, val):
# GH20829
t = Timedelta('1s')
with pytest.raises(TypeError):
t >= val
with pytest.raises(TypeError):
t > val
with pytest.raises(TypeError):
t <= val
with pytest.raises(TypeError):
t < val


class TestTimedeltas(object):

Expand Down

0 comments on commit fce8cc2

Please sign in to comment.