From 5d78297ee674c88157cf4404a3d3b7de262e9e4c Mon Sep 17 00:00:00 2001 From: Victor Date: Fri, 8 Jun 2018 17:09:03 -0300 Subject: [PATCH 01/11] Return NotImplemented for unknown types in comparison. --- pandas/_libs/tslibs/timedeltas.pyx | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/pandas/_libs/tslibs/timedeltas.pyx b/pandas/_libs/tslibs/timedeltas.pyx index 87dc371195b5b..b414043d4a9df 100644 --- a/pandas/_libs/tslibs/timedeltas.pyx +++ b/pandas/_libs/tslibs/timedeltas.pyx @@ -693,13 +693,7 @@ 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: - return False - elif op == Py_NE: - return True - raise TypeError('Cannot compare type {!r} with type ' \ - '{!r}'.format(type(self).__name__, - type(other).__name__)) + return NotImplemented return cmp_scalar(self.value, ots.value, op) From e65673839949bf4eed714466959921bdf95e4384 Mon Sep 17 00:00:00 2001 From: Victor Date: Fri, 8 Jun 2018 17:19:57 -0300 Subject: [PATCH 02/11] Added tests and updated whatsnew. --- doc/source/whatsnew/v0.23.1.txt | 1 + .../tests/scalar/timedelta/test_timedelta.py | 25 +++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/doc/source/whatsnew/v0.23.1.txt b/doc/source/whatsnew/v0.23.1.txt index 5a1bcce9b5970..2193fd5769854 100644 --- a/doc/source/whatsnew/v0.23.1.txt +++ b/doc/source/whatsnew/v0.23.1.txt @@ -59,6 +59,7 @@ Data-type specific - Bug in :meth:`Series.str.replace()` where the method throws `TypeError` on Python 3.5.2 (:issue: `21078`) - Bug in :class:`Timedelta`: where passing a float with a unit would prematurely round the float precision (:issue: `14156`) +- Bug in :class:`Timedelta`: Comparison with unknown types now return NotImplemented as typically expected (:issue: `20829`) - Bug in :func:`pandas.testing.assert_index_equal` which raised ``AssertionError`` incorrectly, when comparing two :class:`CategoricalIndex` objects with param ``check_categorical=False`` (:issue:`19776`) Sparse diff --git a/pandas/tests/scalar/timedelta/test_timedelta.py b/pandas/tests/scalar/timedelta/test_timedelta.py index 205fdf49d3e91..eb30abf780edc 100644 --- a/pandas/tests/scalar/timedelta/test_timedelta.py +++ b/pandas/tests/scalar/timedelta/test_timedelta.py @@ -103,6 +103,31 @@ def test_compare_timedelta_ndarray(self): expected = np.array([False, False]) tm.assert_numpy_array_equal(result, expected) + def test_custom_comparison_object(self): + # GH20829 + class CustomClass(object): + + def __init__(self, eq_result=None): + self.eq_result = eq_result + + def __eq__(self, other): + if self.eq_result is None: + return NotImplemented + elif self.eq_result: + return True + else: + return False + + t = Timedelta('1s') + + assert not (t == "string") + assert not (t == 1) + assert not (t == CustomClass()) + assert not (t == CustomClass(False)) + + assert t == CustomClass(True) + + class TestTimedeltas(object): From 049dbe9f0a5055bdaaf5ff89f7f85adbe1074513 Mon Sep 17 00:00:00 2001 From: Victor Date: Fri, 8 Jun 2018 17:25:39 -0300 Subject: [PATCH 03/11] Removed excess space LINT --- pandas/tests/scalar/timedelta/test_timedelta.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pandas/tests/scalar/timedelta/test_timedelta.py b/pandas/tests/scalar/timedelta/test_timedelta.py index eb30abf780edc..a04941d7adbdf 100644 --- a/pandas/tests/scalar/timedelta/test_timedelta.py +++ b/pandas/tests/scalar/timedelta/test_timedelta.py @@ -128,7 +128,6 @@ def __eq__(self, other): assert t == CustomClass(True) - class TestTimedeltas(object): @pytest.mark.parametrize("unit, value, expected", [ From 1e64ece1c24c718d595757c98862f79c1d1f2ad4 Mon Sep 17 00:00:00 2001 From: Victor Date: Sat, 9 Jun 2018 16:13:27 -0300 Subject: [PATCH 04/11] TST/BUG Always return NotImplemented. Updated tests to take into account that python 2 does not raise TypeError for uncompatible types. --- pandas/_libs/tslibs/timedeltas.pyx | 10 +--- .../tests/scalar/timedelta/test_timedelta.py | 48 ++++++++++++++----- 2 files changed, 38 insertions(+), 20 deletions(-) diff --git a/pandas/_libs/tslibs/timedeltas.pyx b/pandas/_libs/tslibs/timedeltas.pyx index b414043d4a9df..8a5ccd9aeec06 100644 --- a/pandas/_libs/tslibs/timedeltas.pyx +++ b/pandas/_libs/tslibs/timedeltas.pyx @@ -680,15 +680,7 @@ 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 {!r} with type ' \ - '{!r}'.format(type(self).__name__, - 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]) diff --git a/pandas/tests/scalar/timedelta/test_timedelta.py b/pandas/tests/scalar/timedelta/test_timedelta.py index a04941d7adbdf..da113d1a4ce02 100644 --- a/pandas/tests/scalar/timedelta/test_timedelta.py +++ b/pandas/tests/scalar/timedelta/test_timedelta.py @@ -1,4 +1,5 @@ """ test the scalar Timedelta """ +import sys import pytest import numpy as np @@ -42,8 +43,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 sys.version_info >= (3, 0): + with pytest.raises(TypeError): + left > right assert not left == right assert left != right @@ -107,25 +110,48 @@ def test_custom_comparison_object(self): # GH20829 class CustomClass(object): - def __init__(self, eq_result=None): - self.eq_result = eq_result + def __init__(self, cmp_result=None): + self.cmp_result = cmp_result - def __eq__(self, other): - if self.eq_result is None: + def generic_result(self): + if self.cmp_result is None: return NotImplemented - elif self.eq_result: - return True else: - return False + 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(False)) + 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) - assert t == CustomClass(True) + @pytest.mark.parametrize("val", [ + "string", 1]) + def test_raise_comparisons_unknown_types(self, val): + # GH20829 + t = Timedelta('1s') + if sys.version >= (3, 0): + # python 2 does not raises TypeError for comparisons of different types + 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): From adec6594632627cac5422deb92295774a53b61e8 Mon Sep 17 00:00:00 2001 From: Victor Date: Sat, 9 Jun 2018 16:16:54 -0300 Subject: [PATCH 05/11] Line too long. --- pandas/tests/scalar/timedelta/test_timedelta.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pandas/tests/scalar/timedelta/test_timedelta.py b/pandas/tests/scalar/timedelta/test_timedelta.py index da113d1a4ce02..f0238644c42f7 100644 --- a/pandas/tests/scalar/timedelta/test_timedelta.py +++ b/pandas/tests/scalar/timedelta/test_timedelta.py @@ -143,7 +143,8 @@ def test_raise_comparisons_unknown_types(self, val): # GH20829 t = Timedelta('1s') if sys.version >= (3, 0): - # python 2 does not raises TypeError for comparisons of different types + # python 2 does not raises TypeError for comparisons + # of different types with pytest.raises(TypeError): t >= val with pytest.raises(TypeError): From dd185e6521a8d7fa0db291dff337aa6891976f90 Mon Sep 17 00:00:00 2001 From: Victor Date: Sat, 9 Jun 2018 20:07:54 -0300 Subject: [PATCH 06/11] Fixed version_info call in test. --- pandas/tests/scalar/timedelta/test_timedelta.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/tests/scalar/timedelta/test_timedelta.py b/pandas/tests/scalar/timedelta/test_timedelta.py index f0238644c42f7..374783b1c64e1 100644 --- a/pandas/tests/scalar/timedelta/test_timedelta.py +++ b/pandas/tests/scalar/timedelta/test_timedelta.py @@ -142,7 +142,7 @@ def __gt__(self, other): def test_raise_comparisons_unknown_types(self, val): # GH20829 t = Timedelta('1s') - if sys.version >= (3, 0): + if sys.version_info >= (3, 0): # python 2 does not raises TypeError for comparisons # of different types with pytest.raises(TypeError): From 782568fb651b38b491786aa0c1dcd228ea7aeb0e Mon Sep 17 00:00:00 2001 From: victor Date: Sun, 1 Jul 2018 13:51:20 +0200 Subject: [PATCH 07/11] Moved changes to v0.24.0 --- doc/source/whatsnew/v0.23.1.txt | 3 --- doc/source/whatsnew/v0.24.0.txt | 1 + 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/doc/source/whatsnew/v0.23.1.txt b/doc/source/whatsnew/v0.23.1.txt index 9f47edda62725..af4eeffd87d01 100644 --- a/doc/source/whatsnew/v0.23.1.txt +++ b/doc/source/whatsnew/v0.23.1.txt @@ -99,9 +99,6 @@ Bug Fixes - Bug in :meth:`Series.str.replace()` where the method throws `TypeError` on Python 3.5.2 (:issue:`21078`) - Bug in :class:`Timedelta` where passing a float with a unit would prematurely round the float precision (:issue:`14156`) -- Bug in :meth:`Series.str.replace()` where the method throws `TypeError` on Python 3.5.2 (:issue: `21078`) -- Bug in :class:`Timedelta`: where passing a float with a unit would prematurely round the float precision (:issue: `14156`) -- Bug in :class:`Timedelta`: Comparison with unknown types now return NotImplemented as typically expected (:issue: `20829`) - Bug in :func:`pandas.testing.assert_index_equal` which raised ``AssertionError`` incorrectly, when comparing two :class:`CategoricalIndex` objects with param ``check_categorical=False`` (:issue:`19776`) **Sparse** diff --git a/doc/source/whatsnew/v0.24.0.txt b/doc/source/whatsnew/v0.24.0.txt index 0ca5b9cdf1d57..b795495344a7c 100644 --- a/doc/source/whatsnew/v0.24.0.txt +++ b/doc/source/whatsnew/v0.24.0.txt @@ -135,6 +135,7 @@ Datetimelike API Changes - For :class:`DatetimeIndex` and :class:`TimedeltaIndex` with non-``None`` ``freq`` attribute, addition or subtraction of integer-dtyped array or ``Index`` will return an object of the same class (:issue:`19959`) - :class:`DateOffset` objects are now immutable. Attempting to alter one of these will now raise ``AttributeError`` (:issue:`21341`) - :class:`PeriodIndex` subtraction of another ``PeriodIndex`` will now return an object-dtype :class:`Index` of :class:`DateOffset` objects instead of raising a ``TypeError`` (:issue:`20049`) +- Comparing :class:`Timedelta` with unknown types now return NotImplemented instead of False (:issue:`20829`) .. _whatsnew_0240.api.other: From ba307b4b9d09276faa428d8cf5e8b0d4261bb8b0 Mon Sep 17 00:00:00 2001 From: victor Date: Sun, 1 Jul 2018 13:56:25 +0200 Subject: [PATCH 08/11] Skipped test based on PY2 --- .../tests/scalar/timedelta/test_timedelta.py | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/pandas/tests/scalar/timedelta/test_timedelta.py b/pandas/tests/scalar/timedelta/test_timedelta.py index 1b98ea1b9dfaf..0107bb814937c 100644 --- a/pandas/tests/scalar/timedelta/test_timedelta.py +++ b/pandas/tests/scalar/timedelta/test_timedelta.py @@ -44,7 +44,7 @@ def test_ops_error_str(self): left + right # GH 20829: python 2 comparison naturally does not raise TypeError - if sys.version_info >= (3, 0): + if compat.PY3: with pytest.raises(TypeError): left > right @@ -137,22 +137,22 @@ def __gt__(self, other): 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_raise_comparisons_unknown_types(self, val): # GH20829 t = Timedelta('1s') - if sys.version_info >= (3, 0): - # python 2 does not raises TypeError for comparisons - # of different types - with pytest.raises(TypeError): - t >= val - with pytest.raises(TypeError): - t > val - with pytest.raises(TypeError): - t <= val - with pytest.raises(TypeError): - t < val + 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): From 81648ed22fed8dbcd3072f078a998a570b11a93c Mon Sep 17 00:00:00 2001 From: victor Date: Sun, 1 Jul 2018 13:57:39 +0200 Subject: [PATCH 09/11] Updated test names. --- pandas/tests/scalar/timedelta/test_timedelta.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pandas/tests/scalar/timedelta/test_timedelta.py b/pandas/tests/scalar/timedelta/test_timedelta.py index 0107bb814937c..119fc31f1791f 100644 --- a/pandas/tests/scalar/timedelta/test_timedelta.py +++ b/pandas/tests/scalar/timedelta/test_timedelta.py @@ -106,7 +106,7 @@ def test_compare_timedelta_ndarray(self): expected = np.array([False, False]) tm.assert_numpy_array_equal(result, expected) - def test_custom_comparison_object(self): + def test_compare_custom_object(self): # GH20829 class CustomClass(object): @@ -142,7 +142,7 @@ def __gt__(self, other): comparisons of different types") @pytest.mark.parametrize("val", [ "string", 1]) - def test_raise_comparisons_unknown_types(self, val): + def test_compare_unknown_type(self, val): # GH20829 t = Timedelta('1s') with pytest.raises(TypeError): From aa766f034b6874634ccb7550eedbb718c12f6db3 Mon Sep 17 00:00:00 2001 From: victor Date: Sun, 1 Jul 2018 15:48:37 +0200 Subject: [PATCH 10/11] Removed unused import --- pandas/tests/scalar/timedelta/test_timedelta.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pandas/tests/scalar/timedelta/test_timedelta.py b/pandas/tests/scalar/timedelta/test_timedelta.py index 119fc31f1791f..5e44bfdd154de 100644 --- a/pandas/tests/scalar/timedelta/test_timedelta.py +++ b/pandas/tests/scalar/timedelta/test_timedelta.py @@ -1,5 +1,4 @@ """ test the scalar Timedelta """ -import sys import pytest import numpy as np From 16c8222dd7f9b12653c93b2b9a14c12e22e59608 Mon Sep 17 00:00:00 2001 From: Victor Date: Mon, 8 Oct 2018 15:52:55 +0200 Subject: [PATCH 11/11] Fixed whatsnew placement and commented test. --- doc/source/whatsnew/v0.24.0.txt | 4 ++-- pandas/tests/scalar/timedelta/test_timedelta.py | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/doc/source/whatsnew/v0.24.0.txt b/doc/source/whatsnew/v0.24.0.txt index 2ae4d998de6bd..716dab095cbd0 100644 --- a/doc/source/whatsnew/v0.24.0.txt +++ b/doc/source/whatsnew/v0.24.0.txt @@ -186,6 +186,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`) @@ -479,7 +480,6 @@ all-``NaT``. This is for compatibility with ``TimedeltaIndex`` and Previous Behavior: .. code-block:: ipython -- Comparing :class:`Timedelta` with unknown types now return NotImplemented instead of False (:issue:`20829`) In [4]: df = pd.DataFrame([pd.Timedelta(days=1)]) @@ -699,7 +699,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 ^^^^^^^^^ diff --git a/pandas/tests/scalar/timedelta/test_timedelta.py b/pandas/tests/scalar/timedelta/test_timedelta.py index e6dc2489c27f1..0cac1119f76b5 100644 --- a/pandas/tests/scalar/timedelta/test_timedelta.py +++ b/pandas/tests/scalar/timedelta/test_timedelta.py @@ -106,7 +106,8 @@ def test_compare_timedelta_ndarray(self): tm.assert_numpy_array_equal(result, expected) def test_compare_custom_object(self): - # GH20829 + """Make sure non supported operations on Timedelta returns NonImplemented + and yields to other operand (GH20829).""" class CustomClass(object): def __init__(self, cmp_result=None):