BUG: Invalid Timedelta op may raise ValueError

Author: sinhrks <[email protected]>

Closes #13624 from sinhrks/timedelta_comp and squashes the following commits:

856df95 [sinhrks] BUG: Invalid Timedelta op may raise ValueError

Author: sinhrks

doc/source/whatsnew/v0.19.0.txt

@@ -543,7 +543,7 @@ Bug Fixes
 - Bug in ``.to_html``, ``.to_latex`` and ``.to_string`` silently ignore custom datetime formatter passed through the ``formatters`` key word (:issue:`10690`)
 
 - Bug in ``pd.to_numeric`` when ``errors='coerce'`` and input contains non-hashable objects (:issue:`13324`)
-
+- Bug in invalid ``Timedelta`` arithmetic and comparison may raise ``ValueError`` rather than ``TypeError`` (:issue:`13624`)
 
 - Bug in ``Categorical.remove_unused_categories()`` changes ``.codes`` dtype to platform int (:issue:`13261`)
 - Bug in ``groupby`` with ``as_index=False`` returns all NaN's when grouping on multiple columns including a categorical one (:issue:`13204`)

pandas/tseries/tdi.py

@@ -35,16 +35,20 @@ def _td_index_cmp(opname, nat_result=False):
     """
 
     def wrapper(self, other):
+        msg = "cannot compare a TimedeltaIndex with type {0}"
         func = getattr(super(TimedeltaIndex, self), opname)
         if _is_convertible_to_td(other) or other is tslib.NaT:
-            other = _to_m8(other)
+            try:
+                other = _to_m8(other)
+            except ValueError:
+                # failed to parse as timedelta
+                raise TypeError(msg.format(type(other)))
             result = func(other)
             if com.isnull(other):
                 result.fill(nat_result)
         else:
             if not com.is_list_like(other):
-                raise TypeError("cannot compare a TimedeltaIndex with type "
-                                "{0}".format(type(other)))
+                raise TypeError(msg.format(type(other)))
 
             other = TimedeltaIndex(other).values
             result = func(other)

pandas/tseries/tests/test_timedeltas.py

@@ -472,6 +472,21 @@ class Other:
         self.assertTrue(td.__mul__(other) is NotImplemented)
         self.assertTrue(td.__floordiv__(td) is NotImplemented)
 
+    def test_ops_error_str(self):
+        # GH 13624
+        td = Timedelta('1 day')
+
+        for l, r in [(td, 'a'), ('a', td)]:
+
+            with tm.assertRaises(TypeError):
+                l + r
+
+            with tm.assertRaises(TypeError):
+                l > r
+
+            self.assertFalse(l == r)
+            self.assertTrue(l != r)
+
     def test_fields(self):
         def check(value):
             # that we are int/long like
@@ -1432,6 +1447,23 @@ def test_comparisons_nat(self):
             expected = np.array([True, True, True, True, True, False])
             self.assert_numpy_array_equal(result, expected)
 
+    def test_ops_error_str(self):
+        # GH 13624
+        tdi = TimedeltaIndex(['1 day', '2 days'])
+
+        for l, r in [(tdi, 'a'), ('a', tdi)]:
+            with tm.assertRaises(TypeError):
+                l + r
+
+            with tm.assertRaises(TypeError):
+                l > r
+
+            with tm.assertRaises(TypeError):
+                l == r
+
+            with tm.assertRaises(TypeError):
+                l != r
+
     def test_map(self):
 
         rng = timedelta_range('1 day', periods=10)

pandas/tseries/timedeltas.py

@@ -74,8 +74,8 @@ def _convert_listlike(arg, box, unit, name=None):
             value = arg.astype('timedelta64[{0}]'.format(
                 unit)).astype('timedelta64[ns]', copy=False)
         else:
-            value = tslib.array_to_timedelta64(
-                _ensure_object(arg), unit=unit, errors=errors)
+            value = tslib.array_to_timedelta64(_ensure_object(arg),
+                                               unit=unit, errors=errors)
             value = value.astype('timedelta64[ns]', copy=False)
 
         if box:

pandas/tslib.pyx

@@ -2912,10 +2912,17 @@ class Timedelta(_Timedelta):
             if not self._validate_ops_compat(other):
                 return NotImplemented
 
-            other = Timedelta(other)
             if other is NaT:
                 return NaT
+
+            try:
+                other = Timedelta(other)
+            except ValueError:
+                # failed to parse as timedelta
+                return NotImplemented
+
             return Timedelta(op(self.value, other.value), unit='ns')
+
         f.__name__ = name
         return f