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

doc/source/whatsnew/v0.24.0.txt

@@ -1132,9 +1132,9 @@ Timedelta
 - 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:`TimedeltaIndex` where adding a timezone-aware datetime scalar incorrectly returned a timezone-naive :class:`DatetimeIndex` (:issue:`23215`)
 - Bug in :class:`TimedeltaIndex` where adding ``np.timedelta64('NaT')`` incorrectly returned an all-`NaT` :class:`DatetimeIndex` instead of an all-`NaT` :class:`TimedeltaIndex` (:issue:`23215`)
+- 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`)
 - Bug in :class:`Timedelta` and :func:`to_timedelta()` have inconsistencies in supported unit string (:issue:`21762`)
 
-
 Timezones
 ^^^^^^^^^
 

pandas/_libs/tslibs/timedeltas.pxd

@@ -3,6 +3,7 @@
 from numpy cimport int64_t
 
 # Exposed for tslib, not intended for outside use.
+cpdef parse_timedelta_string(object ts, object specified_unit=*)
 cdef 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

@@ -165,6 +165,8 @@ cpdef convert_to_timedelta64(object ts, object unit):
     # kludgy here until we have a timedelta scalar
     # handle the numpy < 1.7 case
     """
+    if unit is None:
+        unit = 'ns'
     if checknull_with_nat(ts):
         return np.timedelta64(NPY_NAT)
     elif isinstance(ts, Timedelta):
@@ -214,7 +216,7 @@ cpdef convert_to_timedelta64(object ts, object unit):
 
 @cython.boundscheck(False)
 @cython.wraparound(False)
-def array_to_timedelta64(object[:] values, unit='ns', errors='raise'):
+def array_to_timedelta64(object[:] values, unit=None, errors='raise'):
     """
     Convert an ndarray to an array of timedeltas. If errors == 'coerce',
     coerce non-convertible objects to NaT. Otherwise, raise.
@@ -237,7 +239,7 @@ def 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:
         unit = parse_timedelta_unit(unit)
         for i in range(n):
@@ -313,7 +315,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.
@@ -427,6 +429,17 @@ 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 were doubly specified, both as an argument ({})"
+                " and inside string ({})".format(specified_unit, unit)
+            )
+
     # we had a dot, but we have a fractional
     # value since we have an unit
     if have_dot and len(unit):
@@ -468,14 +481,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
 
@@ -524,10 +535,9 @@ cpdef inline object parse_timedelta_unit(object unit):
     ----------
     unit : an unit string
     """
-    if unit is None:
-        return 'ns'
-    elif unit == 'M':
+    if unit is None or unit == 'M':
         return unit
+
     try:
         return timedelta_abbrevs[unit.lower()]
     except (KeyError, AttributeError):
@@ -1156,7 +1166,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

@@ -7,7 +7,8 @@
 from pandas._libs import tslibs
 from pandas._libs.tslibs.timedeltas import (convert_to_timedelta64,
                                             array_to_timedelta64,
-                                            parse_timedelta_unit)
+                                            parse_timedelta_unit,
+                                            parse_timedelta_string)
 
 from pandas.core.dtypes.common import (
     ensure_object,
@@ -17,7 +18,7 @@
 from pandas.core.dtypes.generic import ABCSeries, ABCIndexClass
 
 
-def to_timedelta(arg, unit='ns', box=True, errors='raise'):
+def to_timedelta(arg, unit=None, box=True, errors='raise'):
     """
     Convert argument to timedelta
 
@@ -106,26 +107,30 @@ def to_timedelta(arg, unit='ns', box=True, errors='raise'):
                                             box=box, errors=errors)
 
 
-def _coerce_scalar_to_timedelta_type(r, unit='ns', box=True, errors='raise'):
+def _coerce_scalar_to_timedelta_type(r, unit=None, 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
+        result = parse_timedelta_string(r, unit)
+        result = np.timedelta64(result)
+    except (ValueError, TypeError):
+        try:
+            result = convert_to_timedelta64(r, unit)
+        except ValueError:
+            if errors == 'raise':
+                raise
+            elif errors == 'ignore':
+                return r
 
-        # coerce
-        result = pd.NaT
+            # coerce
+            result = pd.NaT
 
     if box:
         result = tslibs.Timedelta(result)
     return result
 
 
-def _convert_listlike(arg, unit='ns', box=True, errors='raise', name=None):
+def _convert_listlike(arg, unit=None, box=True, errors='raise', name=None):
     """Convert a list of objects to a timedelta index object."""
 
     if isinstance(arg, (list, tuple)) or not hasattr(arg, 'dtype'):
@@ -135,6 +140,8 @@ def _convert_listlike(arg, unit='ns', box=True, errors='raise', name=None):
     if is_timedelta64_dtype(arg):
         value = arg.astype('timedelta64[ns]')
     elif is_integer_dtype(arg):
+        if unit is None:
+            unit = 'ns'
         value = arg.astype('timedelta64[{unit}]'.format(unit=unit)).astype(
             'timedelta64[ns]', copy=False)
     else:

pandas/tests/frame/test_arithmetic.py

@@ -299,7 +299,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})
 
@@ -560,7 +560,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

@@ -232,7 +232,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/indexes/timedeltas/test_partial_slicing.py

@@ -52,7 +52,8 @@ def test_partial_slice_high_reso(self):
         assert result == s.iloc[1001]
 
     def test_slice_with_negative_step(self):
-        ts = Series(np.arange(20), timedelta_range('0', periods=20, freq='H'))
+        ts = Series(np.arange(20),
+                    timedelta_range('0ns', periods=20, freq='H'))
         SLC = pd.IndexSlice
 
         def assert_slices_equivalent(l_slc, i_slc):
@@ -77,7 +78,8 @@ def assert_slices_equivalent(l_slc, i_slc):
         assert_slices_equivalent(SLC['7 hours':'15 hours':-1], SLC[:0])
 
     def test_slice_with_zero_step_raises(self):
-        ts = Series(np.arange(20), timedelta_range('0', periods=20, freq='H'))
+        ts = Series(np.arange(20),
+                    timedelta_range('0ns', periods=20, freq='H'))
         tm.assert_raises_regex(ValueError, 'slice step cannot be zero',
                                lambda: ts[::0])
         tm.assert_raises_regex(ValueError, 'slice step cannot be zero',

pandas/tests/plotting/test_datetimelike.py

@@ -1401,7 +1401,7 @@ def test_format_timedelta_ticks_narrow(self):
                 '00:00:00.00000000{:d}'.format(2 * i)
                 for i in range(5)] + ['']
 
-        rng = timedelta_range('0', periods=10, freq='ns')
+        rng = timedelta_range('0ns', periods=10, freq='ns')
         df = DataFrame(np.random.randn(len(rng), 3), rng)
         fig, ax = self.plt.subplots()
         df.plot(fontsize=2, ax=ax)
@@ -1432,7 +1432,7 @@ def test_format_timedelta_ticks_wide(self):
             expected_labels = expected_labels[1:-1]
             expected_labels[-1] = ''
 
-        rng = timedelta_range('0', periods=10, freq='1 d')
+        rng = timedelta_range('0ns', periods=10, freq='1 d')
         df = DataFrame(np.random.randn(len(rng), 3), rng)
         fig, ax = self.plt.subplots()
         ax = df.plot(fontsize=2, ax=ax)

pandas/tests/scalar/timedelta/test_construction.py

@@ -85,10 +85,6 @@ def test_construction():
     with pytest.raises(ValueError):
         Timedelta('10 days -1 h 1.5m 1s 3us')
 
-    # no units specified
-    with pytest.raises(ValueError):
-        Timedelta('3.1415')
-
     # invalid construction
     tm.assert_raises_regex(ValueError, "cannot construct a Timedelta",
                            lambda: Timedelta())
@@ -210,3 +206,45 @@ 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("value", [
+    3.1415,  # Number with decimals (original test)
+    10,      # Integer number, did not raise before
+])
+@pytest.mark.parametrize("str_unit, unit, expectation", [
+    # Expected case
+    ("",
+     "s",
+     tm.do_not_raise),
+
+    # Units doubly defined
+    ("s",
+     "d",
+     pytest.raises(ValueError,
+                   message="units were doubly specified, "
+                           "both as an argument (d) and inside string (s)")
+     ),
+
+    # Units doubly defined (same)
+    ("s",
+     "s",
+     pytest.raises(ValueError,
+                   message="units were doubly specified, "
+                           "both as an argument (s) and inside string (s)")),
+
+    # No units
+    ("",
+     None,
+     pytest.raises(ValueError,
+                   message="number string without units")),
+])
+def test_string_with_unit(value, str_unit, unit, expectation):
+    with expectation:
+        val_str = "{}{}".format(value, str_unit)
+        expected_td = Timedelta(value, unit=unit)
+
+        assert Timedelta(val_str, unit=unit) == expected_td
+        assert pd.to_timedelta(val_str, unit=unit) == expected_td
+        assert all(pd.to_timedelta([val_str, val_str], unit=unit) ==
+                   pd.to_timedelta([expected_td, expected_td]))

pandas/tests/scalar/timedelta/test_timedelta.py

@@ -473,12 +473,9 @@ def test_short_format_converters(self):
         def conv(v):
             return v.astype('m8[ns]')
 
-        assert ct('10') == np.timedelta64(10, 'ns')
         assert ct('10ns') == np.timedelta64(10, 'ns')
-        assert ct('100') == np.timedelta64(100, 'ns')
         assert ct('100ns') == np.timedelta64(100, 'ns')
 
-        assert ct('1000') == np.timedelta64(1000, 'ns')
         assert ct('1000ns') == np.timedelta64(1000, 'ns')
         assert ct('1000NS') == np.timedelta64(1000, 'ns')
 
@@ -615,7 +612,7 @@ def test_implementation_limits(self):
     def test_total_seconds_precision(self):
         # GH 19458
         assert Timedelta('30S').total_seconds() == 30.0
-        assert Timedelta('0').total_seconds() == 0.0
+        assert Timedelta('0ns').total_seconds() == 0.0
         assert Timedelta('-2S').total_seconds() == -2.0
         assert Timedelta('5.324S').total_seconds() == 5.324
         assert (Timedelta('30S').total_seconds() - 30.0) < 1e-20

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

@@ -40,6 +40,42 @@
 from pandas.io.common import urlopen
 from pandas.io.formats.printing import pprint_thing
 
+
+class NullContextManager(object):
+    """No-op context manager (does nothing).
+
+    Mainly used for defining ``do_not_raise`` context manager,
+    for pytest tests where we are required to parametrize on
+    whether we should raise something or not:
+
+    Example
+    -------
+
+        >>> import pytest
+        >>> from pytest import raises
+        >>> from pandas.util.testing import do_not_raise
+        >>> @pytest.mark.parametrize("input, expectation", [
+        ...     (1, do_not_raise),
+        ...     ("a", raises(ValueError))
+        ... ])
+        >>> def test_convert_number(input, expectation):
+        ...     with expectation:
+        ...         float(input)
+
+    """
+    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