Accepts integer/float string with units and raises when unit is ambiguous (2)

doc/source/whatsnew/v0.24.0.txt

@@ -698,7 +698,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`)
--
+- Bug in :class:`Timedelta`: where passing a string of a pure number would not take unit into account. Also raises for ambiguous/duplicate unit specification (:issue: `12136`)
 
 Timezones
 ^^^^^^^^^

pandas/_libs/tslibs/timedeltas.pxd

@@ -3,7 +3,7 @@
 from numpy cimport int64_t
 
 # Exposed for tslib, not intended for outside use.
-cdef parse_timedelta_string(object ts)
+cpdef parse_timedelta_string(object ts, specified_unit=*)
 cpdef int64_t cast_from_unit(object ts, object unit) except? -1
 cpdef int64_t delta_to_nanoseconds(delta) except? -1
 cpdef convert_to_timedelta64(object ts, object unit)

pandas/_libs/tslibs/timedeltas.pyx

@@ -221,7 +221,7 @@ cpdef array_to_timedelta64(object[:] values, unit='ns', errors='raise'):
     # this is where all of the error handling will take place.
     try:
         for i in range(n):
-            result[i] = parse_timedelta_string(values[i])
+            result[i] = parse_timedelta_string(values[i], specified_unit=unit)
     except:
         for i in range(n):
             try:
@@ -287,7 +287,7 @@ cdef inline _decode_if_necessary(object ts):
     return ts
 
 
-cdef inline parse_timedelta_string(object ts):
+cpdef inline parse_timedelta_string(object ts, specified_unit=None):
     """
     Parse a regular format timedelta string. Return an int64_t (in ns)
     or raise a ValueError on an invalid parse.
@@ -401,6 +401,14 @@ cdef inline parse_timedelta_string(object ts):
             have_value = 1
             have_dot = 0
 
+    # Consider units from outside
+    if not unit:
+        if specified_unit:
+            unit = specified_unit
+    else:
+        if specified_unit:
+            raise ValueError("units doubly specified")
+
     # we had a dot, but we have a fractional
     # value since we have an unit
     if have_dot and len(unit):
@@ -442,14 +450,12 @@ cdef inline parse_timedelta_string(object ts):
         else:
             raise ValueError("unit abbreviation w/o a number")
 
-    # treat as nanoseconds
-    # but only if we don't have anything else
+    # raise if we just have a number without units
     else:
         if have_value:
             raise ValueError("have leftover units")
         if len(number):
-            r = timedelta_from_spec(number, frac, 'ns')
-            result += timedelta_as_neg(r, neg)
+            raise ValueError("number string without units")
 
     return result
 
@@ -1119,7 +1125,7 @@ class Timedelta(_Timedelta):
             if len(value) > 0 and value[0] == 'P':
                 value = parse_iso_format_string(value)
             else:
-                value = parse_timedelta_string(value)
+                value = parse_timedelta_string(value, specified_unit=unit)
             value = np.timedelta64(value)
         elif PyDelta_Check(value):
             value = convert_to_timedelta64(value, 'ns')

pandas/core/tools/timedeltas.py

@@ -6,7 +6,8 @@
 import pandas as pd
 from pandas._libs import tslibs
 from pandas._libs.tslibs.timedeltas import (convert_to_timedelta64,
-                                            array_to_timedelta64)
+                                            array_to_timedelta64,
+                                            parse_timedelta_string)
 
 from pandas.core.dtypes.common import (
     ensure_object,
@@ -142,15 +143,19 @@ def _coerce_scalar_to_timedelta_type(r, unit='ns', box=True, errors='raise'):
     """Convert string 'r' to a timedelta object."""
 
     try:
-        result = convert_to_timedelta64(r, unit)
-    except ValueError:
-        if errors == 'raise':
-            raise
-        elif errors == 'ignore':
-            return r
-
-        # coerce
-        result = pd.NaT
+        result = parse_timedelta_string(r)
+        result = np.timedelta64(result)
+    except Exception:
+        try:
+            result = convert_to_timedelta64(r, unit)
+        except ValueError:
+            if errors == 'raise':
+                raise
+            elif errors == 'ignore':
+                return r
+
+            # coerce
+            result = pd.NaT
 
     if box:
         result = tslibs.Timedelta(result)

pandas/tests/frame/test_arithmetic.py

@@ -102,7 +102,7 @@ class TestFrameFlexArithmetic(object):
     def test_df_add_td64_columnwise(self):
         # GH#22534 Check that column-wise addition broadcasts correctly
         dti = pd.date_range('2016-01-01', periods=10)
-        tdi = pd.timedelta_range('1', periods=10)
+        tdi = pd.timedelta_range('1ns', periods=10)
         tser = pd.Series(tdi)
         df = pd.DataFrame({0: dti, 1: tdi})
 
@@ -270,7 +270,7 @@ def test_df_bool_mul_int(self):
     def test_td64_df_add_int_frame(self):
         # GH#22696 Check that we don't dispatch to numpy implementation,
         # which treats int64 as m8[ns]
-        tdi = pd.timedelta_range('1', periods=3)
+        tdi = pd.timedelta_range('1ns', periods=3)
         df = tdi.to_frame()
         other = pd.DataFrame([1, 2, 3], index=tdi)  # indexed like `df`
         with pytest.raises(TypeError):

pandas/tests/indexes/timedeltas/test_ops.py

@@ -234,7 +234,7 @@ def test_drop_duplicates(self):
                                       'T', '2T', 'S', '-3S'])
     def test_infer_freq(self, freq):
         # GH#11018
-        idx = pd.timedelta_range('1', freq=freq, periods=10)
+        idx = pd.timedelta_range('1ns', freq=freq, periods=10)
         result = pd.TimedeltaIndex(idx.asi8, freq='infer')
         tm.assert_index_equal(idx, result)
         assert result.freq == freq

pandas/tests/scalar/timedelta/test_construction.py

@@ -89,6 +89,9 @@ def test_construction():
     with pytest.raises(ValueError):
         Timedelta('3.1415')
 
+    with pytest.raises(ValueError):
+        Timedelta('2000')
+
     # invalid construction
     tm.assert_raises_regex(ValueError, "cannot construct a Timedelta",
                            lambda: Timedelta())
@@ -210,3 +213,20 @@ def test_td_constructor_on_nanoseconds(constructed_td, conversion):
 def test_td_constructor_value_error():
     with pytest.raises(TypeError):
         Timedelta(nanoseconds='abc')
+
+
+@pytest.mark.parametrize("str_unit, unit, expectation", [
+    ("", "s", tm.do_not_raise),              # Expected case
+    ("s", "s", pytest.raises(ValueError)),   # Units doubly defined
+    ("s", "d", pytest.raises(ValueError)),
+    ("", None, pytest.raises(ValueError)),   # No units
+])
+def test_string_with_unit(str_unit, unit, expectation):
+    with expectation:
+        val_str = "10{}".format(str_unit)
+        expected_td = Timedelta(10, unit=unit)
+
+        assert Timedelta(val_str, unit=unit) == expected_td
+        assert pd.to_timedelta(val_str, unit=unit) == expected_td
+        assert pd.to_timedelta([val_str, val_str], unit=unit) == \
+            pd.to_timedelta([expected_td, expected_td])

pandas/tests/test_resample.py

@@ -999,7 +999,7 @@ def test_resample_single_period_timedelta(self):
     def test_resample_timedelta_idempotency(self):
 
         # GH 12072
-        index = pd.timedelta_range('0', periods=9, freq='10L')
+        index = pd.timedelta_range('0ns', periods=9, freq='10L')
         series = Series(range(9), index=index)
         result = series.resample('10L').mean()
         expected = series

pandas/tests/util/test_hashing.py

@@ -20,7 +20,7 @@ class TestHashing(object):
         Series([True, False, True] * 3),
         Series(pd.date_range('20130101', periods=9)),
         Series(pd.date_range('20130101', periods=9, tz='US/Eastern')),
-        Series(pd.timedelta_range('2000', periods=9))])
+        Series(pd.timedelta_range('2000ns', periods=9))])
     def series(self, request):
         return request.param
 

pandas/util/testing.py

@@ -49,6 +49,20 @@
 from pandas.io.common import urlopen
 
 
+class NullContextManager(object):
+    def __init__(self, dummy_resource=None):
+        self.dummy_resource = dummy_resource
+
+    def __enter__(self):
+        return self.dummy_resource
+
+    def __exit__(self, *args):
+        pass
+
+
+do_not_raise = NullContextManager()
+
+
 N = 30
 K = 4
 _RAISE_NETWORK_ERROR_DEFAULT = False