From ab5f7ba2a80ec427888ff3decbe4d9c972aeac27 Mon Sep 17 00:00:00 2001 From: Victor Date: Fri, 5 Oct 2018 16:22:28 +0200 Subject: [PATCH 01/38] Added null context manager as do_not_raise for parametrization in pytest. --- pandas/util/testing.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/pandas/util/testing.py b/pandas/util/testing.py index 4e01e0feb004c..9c58830d6b200 100644 --- a/pandas/util/testing.py +++ b/pandas/util/testing.py @@ -49,6 +49,18 @@ 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 From 7a38ecdffcb1826ac508c9b64c74603f843e3d12 Mon Sep 17 00:00:00 2001 From: Victor Date: Fri, 5 Oct 2018 16:23:04 +0200 Subject: [PATCH 02/38] Added test for pure integer strings and test for strings with/without units. --- .../tests/scalar/timedelta/test_construction.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/pandas/tests/scalar/timedelta/test_construction.py b/pandas/tests/scalar/timedelta/test_construction.py index d648140aa7347..a167ceed7059a 100644 --- a/pandas/tests/scalar/timedelta/test_construction.py +++ b/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,16 @@ 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) + assert Timedelta(val_str, unit=unit) == Timedelta(10, unit=unit) + assert pd.to_timedelta(val_str, unit=unit) == Timedelta(10, unit=unit) From a65a35a44397a4c29a8f1398e4b9bc66c929f0a4 Mon Sep 17 00:00:00 2001 From: Victor Date: Fri, 5 Oct 2018 16:23:17 +0200 Subject: [PATCH 03/38] Added entry to whatsnew. --- doc/source/whatsnew/v0.24.0.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/source/whatsnew/v0.24.0.txt b/doc/source/whatsnew/v0.24.0.txt index 91575c311b409..681b28b4b29af 100644 --- a/doc/source/whatsnew/v0.24.0.txt +++ b/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 ^^^^^^^^^ From 57dea1dea0fc0932a6518aacb6e0c52581c3a413 Mon Sep 17 00:00:00 2001 From: Victor Date: Fri, 5 Oct 2018 16:23:38 +0200 Subject: [PATCH 04/38] Changed Timedelta to raise for strings without unit. --- pandas/_libs/tslibs/timedeltas.pyx | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/pandas/_libs/tslibs/timedeltas.pyx b/pandas/_libs/tslibs/timedeltas.pyx index 9c8be1901d1dc..4a64eb5fa807e 100644 --- a/pandas/_libs/tslibs/timedeltas.pyx +++ b/pandas/_libs/tslibs/timedeltas.pyx @@ -1116,11 +1116,34 @@ class Timedelta(_Timedelta): if isinstance(value, Timedelta): value = value.value elif is_string_object(value): - if len(value) > 0 and value[0] == 'P': + # Check if it is just a number in a string + try: + value = int(value) + except (ValueError, TypeError): + try: + value = float(value) + except (ValueError, TypeError): + pass + + if is_integer_object(value) or is_float_object(value): + value = convert_to_timedelta64(value, unit) + elif len(value) > 0 and value[0] == 'P': value = parse_iso_format_string(value) else: - value = parse_timedelta_string(value) - value = np.timedelta64(value) + try: + orig_value = value + value = float(value) + except ValueError: + if unit is not None: + raise ValueError("Unit cannot be defined for strings other than pure integer/floats." + " Value: {} Unit: {}".format(value, unit)) + value = parse_timedelta_string(value) + value = np.timedelta64(value) + else: + if unit is None: + raise ValueError("Cannot convert float string without unit." + " Value: {} Type: {}".format(orig_value, type(orig_value))) + value = convert_to_timedelta64(value, unit) elif PyDelta_Check(value): value = convert_to_timedelta64(value, 'ns') elif is_timedelta64_object(value): From b8cf952d9d861c6ac6f458a24184e4f0bdef612e Mon Sep 17 00:00:00 2001 From: Victor Date: Sun, 7 Oct 2018 16:06:28 +0200 Subject: [PATCH 05/38] WIP refactoring. --- pandas/_libs/tslibs/timedeltas.pyx | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/pandas/_libs/tslibs/timedeltas.pyx b/pandas/_libs/tslibs/timedeltas.pyx index 4a64eb5fa807e..c59d918dc388e 100644 --- a/pandas/_libs/tslibs/timedeltas.pyx +++ b/pandas/_libs/tslibs/timedeltas.pyx @@ -1116,18 +1116,7 @@ class Timedelta(_Timedelta): if isinstance(value, Timedelta): value = value.value elif is_string_object(value): - # Check if it is just a number in a string - try: - value = int(value) - except (ValueError, TypeError): - try: - value = float(value) - except (ValueError, TypeError): - pass - - if is_integer_object(value) or is_float_object(value): - value = convert_to_timedelta64(value, unit) - elif len(value) > 0 and value[0] == 'P': + if len(value) > 0 and value[0] == 'P': value = parse_iso_format_string(value) else: try: From 81548591f3c20f2e808960d95c0714b321c7c274 Mon Sep 17 00:00:00 2001 From: Victor Date: Sun, 7 Oct 2018 16:32:53 +0200 Subject: [PATCH 06/38] Deprecation warning when we don't have units in string numbers. --- pandas/_libs/tslibs/timedeltas.pyx | 7 ++----- pandas/tests/scalar/timedelta/test_construction.py | 4 ++-- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/pandas/_libs/tslibs/timedeltas.pyx b/pandas/_libs/tslibs/timedeltas.pyx index c59d918dc388e..ce6f43605f0b5 100644 --- a/pandas/_libs/tslibs/timedeltas.pyx +++ b/pandas/_libs/tslibs/timedeltas.pyx @@ -1120,18 +1120,15 @@ class Timedelta(_Timedelta): value = parse_iso_format_string(value) else: try: - orig_value = value value = float(value) except ValueError: if unit is not None: - raise ValueError("Unit cannot be defined for strings other than pure integer/floats." - " Value: {} Unit: {}".format(value, unit)) + raise ValueError("Unit cannot be defined for strings other than pure numbers.") value = parse_timedelta_string(value) value = np.timedelta64(value) else: if unit is None: - raise ValueError("Cannot convert float string without unit." - " Value: {} Type: {}".format(orig_value, type(orig_value))) + warnings.warn("Converting float string without unit is not allowed and will be deprecated.", DeprecationWarning) value = convert_to_timedelta64(value, unit) elif PyDelta_Check(value): value = convert_to_timedelta64(value, 'ns') diff --git a/pandas/tests/scalar/timedelta/test_construction.py b/pandas/tests/scalar/timedelta/test_construction.py index a167ceed7059a..fb9945d4d2bb8 100644 --- a/pandas/tests/scalar/timedelta/test_construction.py +++ b/pandas/tests/scalar/timedelta/test_construction.py @@ -219,10 +219,10 @@ def test_td_constructor_value_error(): ("", "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 + ("", None, pytest.warns(DeprecationWarning)), # No units ]) def test_string_with_unit(str_unit, unit, expectation): with expectation: val_str = "10{}".format(str_unit) assert Timedelta(val_str, unit=unit) == Timedelta(10, unit=unit) - assert pd.to_timedelta(val_str, unit=unit) == Timedelta(10, unit=unit) + #assert pd.to_timedelta(val_str, unit=unit) == Timedelta(10, unit=unit) From a90a992f78e9ce31a5177ca9c95a16b61e01fe02 Mon Sep 17 00:00:00 2001 From: Victor Date: Sun, 7 Oct 2018 16:37:02 +0200 Subject: [PATCH 07/38] Fixed linting. --- pandas/util/testing.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pandas/util/testing.py b/pandas/util/testing.py index 9c58830d6b200..3eb9ac9999cb7 100644 --- a/pandas/util/testing.py +++ b/pandas/util/testing.py @@ -52,8 +52,10 @@ 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 From 1184b6e9fe8b1df88807c63bc2210ccd60f48a6e Mon Sep 17 00:00:00 2001 From: Victor Date: Sun, 7 Oct 2018 16:37:16 +0200 Subject: [PATCH 08/38] Added to_timedelta test. --- pandas/tests/scalar/timedelta/test_construction.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/tests/scalar/timedelta/test_construction.py b/pandas/tests/scalar/timedelta/test_construction.py index fb9945d4d2bb8..36f3de6ea63d6 100644 --- a/pandas/tests/scalar/timedelta/test_construction.py +++ b/pandas/tests/scalar/timedelta/test_construction.py @@ -225,4 +225,4 @@ def test_string_with_unit(str_unit, unit, expectation): with expectation: val_str = "10{}".format(str_unit) assert Timedelta(val_str, unit=unit) == Timedelta(10, unit=unit) - #assert pd.to_timedelta(val_str, unit=unit) == Timedelta(10, unit=unit) + assert pd.to_timedelta(val_str, unit=unit) == Timedelta(10, unit=unit) From 5e28b491228cb218f84a67c167be1253780d12bb Mon Sep 17 00:00:00 2001 From: Victor Date: Sun, 7 Oct 2018 17:18:16 +0200 Subject: [PATCH 09/38] scalar case in to_timedelta checks for strings. --- pandas/core/tools/timedeltas.py | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/pandas/core/tools/timedeltas.py b/pandas/core/tools/timedeltas.py index 4dc4fcb00d84d..efcb134eba201 100644 --- a/pandas/core/tools/timedeltas.py +++ b/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,18 @@ 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) + except: + 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) From fafe838edded3d3b5d3f6cf157e9bcf01fd4e813 Mon Sep 17 00:00:00 2001 From: Victor Date: Sun, 7 Oct 2018 17:18:51 +0200 Subject: [PATCH 10/38] Updated test with more cases for to_timedelta. --- pandas/tests/scalar/timedelta/test_construction.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pandas/tests/scalar/timedelta/test_construction.py b/pandas/tests/scalar/timedelta/test_construction.py index 36f3de6ea63d6..2042fc4df9fe8 100644 --- a/pandas/tests/scalar/timedelta/test_construction.py +++ b/pandas/tests/scalar/timedelta/test_construction.py @@ -224,5 +224,9 @@ def test_td_constructor_value_error(): def test_string_with_unit(str_unit, unit, expectation): with expectation: val_str = "10{}".format(str_unit) - assert Timedelta(val_str, unit=unit) == Timedelta(10, unit=unit) - assert pd.to_timedelta(val_str, unit=unit) == Timedelta(10, unit=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]) From 3ca0dae1bd6f0ef5c2c19394304bf7454902c45b Mon Sep 17 00:00:00 2001 From: Victor Date: Sun, 7 Oct 2018 18:00:00 +0200 Subject: [PATCH 11/38] All tests passing. Implemented checks on parse_timedelta_string. --- pandas/_libs/tslibs/timedeltas.pxd | 2 +- pandas/_libs/tslibs/timedeltas.pyx | 31 +++++++++---------- pandas/core/tools/timedeltas.py | 3 +- .../scalar/timedelta/test_construction.py | 2 +- pandas/tests/util/test_hashing.py | 2 +- 5 files changed, 19 insertions(+), 21 deletions(-) diff --git a/pandas/_libs/tslibs/timedeltas.pxd b/pandas/_libs/tslibs/timedeltas.pxd index eda4418902513..4541a0e070884 100644 --- a/pandas/_libs/tslibs/timedeltas.pxd +++ b/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) diff --git a/pandas/_libs/tslibs/timedeltas.pyx b/pandas/_libs/tslibs/timedeltas.pyx index ce6f43605f0b5..4580341d53f96 100644 --- a/pandas/_libs/tslibs/timedeltas.pyx +++ b/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,17 +1125,8 @@ class Timedelta(_Timedelta): if len(value) > 0 and value[0] == 'P': value = parse_iso_format_string(value) else: - try: - value = float(value) - except ValueError: - if unit is not None: - raise ValueError("Unit cannot be defined for strings other than pure numbers.") - value = parse_timedelta_string(value) - value = np.timedelta64(value) - else: - if unit is None: - warnings.warn("Converting float string without unit is not allowed and will be deprecated.", DeprecationWarning) - value = convert_to_timedelta64(value, unit) + value = parse_timedelta_string(value, specified_unit=unit) + value = np.timedelta64(value) elif PyDelta_Check(value): value = convert_to_timedelta64(value, 'ns') elif is_timedelta64_object(value): diff --git a/pandas/core/tools/timedeltas.py b/pandas/core/tools/timedeltas.py index efcb134eba201..fd3148352cd21 100644 --- a/pandas/core/tools/timedeltas.py +++ b/pandas/core/tools/timedeltas.py @@ -144,7 +144,8 @@ def _coerce_scalar_to_timedelta_type(r, unit='ns', box=True, errors='raise'): try: result = parse_timedelta_string(r) - except: + result = np.timedelta64(result) + except Exception: try: result = convert_to_timedelta64(r, unit) except ValueError: diff --git a/pandas/tests/scalar/timedelta/test_construction.py b/pandas/tests/scalar/timedelta/test_construction.py index 2042fc4df9fe8..439ed1a16313a 100644 --- a/pandas/tests/scalar/timedelta/test_construction.py +++ b/pandas/tests/scalar/timedelta/test_construction.py @@ -219,7 +219,7 @@ def test_td_constructor_value_error(): ("", "s", tm.do_not_raise), # Expected case ("s", "s", pytest.raises(ValueError)), # Units doubly defined ("s", "d", pytest.raises(ValueError)), - ("", None, pytest.warns(DeprecationWarning)), # No units + ("", None, pytest.raises(ValueError)), # No units ]) def test_string_with_unit(str_unit, unit, expectation): with expectation: diff --git a/pandas/tests/util/test_hashing.py b/pandas/tests/util/test_hashing.py index b62260071d996..99a4b9c9c85f6 100644 --- a/pandas/tests/util/test_hashing.py +++ b/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 From 8105f9c161d548a7da54ed4abf8a655f45fe36c8 Mon Sep 17 00:00:00 2001 From: Victor Date: Sun, 7 Oct 2018 18:21:31 +0200 Subject: [PATCH 12/38] Updated tests to avoid creating timedeltas from strings without unit. --- pandas/tests/frame/test_arithmetic.py | 4 ++-- pandas/tests/indexes/timedeltas/test_ops.py | 2 +- pandas/tests/test_resample.py | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pandas/tests/frame/test_arithmetic.py b/pandas/tests/frame/test_arithmetic.py index 2eb11c3a2e2f7..1d436a0e1ad89 100644 --- a/pandas/tests/frame/test_arithmetic.py +++ b/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): diff --git a/pandas/tests/indexes/timedeltas/test_ops.py b/pandas/tests/indexes/timedeltas/test_ops.py index d7bdd18f48523..bc8c34c4a951c 100644 --- a/pandas/tests/indexes/timedeltas/test_ops.py +++ b/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 diff --git a/pandas/tests/test_resample.py b/pandas/tests/test_resample.py index 5cd31e08e0a9b..098813845d087 100644 --- a/pandas/tests/test_resample.py +++ b/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 From e90a18bf7a42b6fa1dfdff9ceebda43fb1ba4381 Mon Sep 17 00:00:00 2001 From: Victor Date: Sun, 7 Oct 2018 19:13:28 +0200 Subject: [PATCH 13/38] Updated more tests to avoid creating timedeltas from strings without unit. --- pandas/tests/indexes/timedeltas/test_ops.py | 2 +- pandas/tests/indexes/timedeltas/test_partial_slicing.py | 4 ++-- pandas/tests/plotting/test_datetimelike.py | 4 ++-- pandas/tests/scalar/timedelta/test_timedelta.py | 4 +--- 4 files changed, 6 insertions(+), 8 deletions(-) diff --git a/pandas/tests/indexes/timedeltas/test_ops.py b/pandas/tests/indexes/timedeltas/test_ops.py index bc8c34c4a951c..744c21603d58a 100644 --- a/pandas/tests/indexes/timedeltas/test_ops.py +++ b/pandas/tests/indexes/timedeltas/test_ops.py @@ -241,7 +241,7 @@ def test_infer_freq(self, freq): def test_nat_new(self): - idx = pd.timedelta_range('1', freq='D', periods=5, name='x') + idx = pd.timedelta_range('1ns', freq='D', periods=5, name='x') result = idx._nat_new() exp = pd.TimedeltaIndex([pd.NaT] * 5, name='x') tm.assert_index_equal(result, exp) diff --git a/pandas/tests/indexes/timedeltas/test_partial_slicing.py b/pandas/tests/indexes/timedeltas/test_partial_slicing.py index 7c5f82193da6d..da8baa43cbb54 100644 --- a/pandas/tests/indexes/timedeltas/test_partial_slicing.py +++ b/pandas/tests/indexes/timedeltas/test_partial_slicing.py @@ -53,7 +53,7 @@ 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): @@ -78,7 +78,7 @@ 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', diff --git a/pandas/tests/plotting/test_datetimelike.py b/pandas/tests/plotting/test_datetimelike.py index de6f6b931987c..93126335753b3 100644 --- a/pandas/tests/plotting/test_datetimelike.py +++ b/pandas/tests/plotting/test_datetimelike.py @@ -1372,7 +1372,7 @@ def test_format_timedelta_ticks_narrow(self): '00:00:00.00000000{:d}'.format(i) for i in range(10)] - 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) @@ -1414,7 +1414,7 @@ def test_format_timedelta_ticks_wide(self): '' ] - 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) diff --git a/pandas/tests/scalar/timedelta/test_timedelta.py b/pandas/tests/scalar/timedelta/test_timedelta.py index 017606dc42d59..d646ecdb2ddb3 100644 --- a/pandas/tests/scalar/timedelta/test_timedelta.py +++ b/pandas/tests/scalar/timedelta/test_timedelta.py @@ -395,12 +395,10 @@ 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') @@ -537,7 +535,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 From e0a55a870fa62eb8339813817bcd70f6e09c539f Mon Sep 17 00:00:00 2001 From: Victor Date: Sun, 7 Oct 2018 20:17:42 +0200 Subject: [PATCH 14/38] Updated more tests to avoid creating timedeltas from strings without unit. --- 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 d646ecdb2ddb3..d705495efc235 100644 --- a/pandas/tests/scalar/timedelta/test_timedelta.py +++ b/pandas/tests/scalar/timedelta/test_timedelta.py @@ -396,7 +396,6 @@ def conv(v): return v.astype('m8[ns]') assert ct('10ns') == np.timedelta64(10, 'ns') - assert ct('100') == np.timedelta64(100, 'ns') assert ct('100ns') == np.timedelta64(100, 'ns') assert ct('1000ns') == np.timedelta64(1000, 'ns') From 827aebfc5f39e89bff7dc89aedaf1dd864d5b9ca Mon Sep 17 00:00:00 2001 From: Victor Date: Sun, 7 Oct 2018 20:41:45 +0200 Subject: [PATCH 15/38] Fixed failing test. --- pandas/_libs/tslibs/timedeltas.pyx | 2 +- pandas/core/tools/timedeltas.py | 2 +- pandas/tests/scalar/timedelta/test_construction.py | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pandas/_libs/tslibs/timedeltas.pyx b/pandas/_libs/tslibs/timedeltas.pyx index 4580341d53f96..29566b9118790 100644 --- a/pandas/_libs/tslibs/timedeltas.pyx +++ b/pandas/_libs/tslibs/timedeltas.pyx @@ -404,7 +404,7 @@ cpdef inline parse_timedelta_string(object ts, specified_unit=None): # Consider units from outside if not unit: if specified_unit: - unit = specified_unit + unit = [specified_unit] else: if specified_unit: raise ValueError("units doubly specified") diff --git a/pandas/core/tools/timedeltas.py b/pandas/core/tools/timedeltas.py index fd3148352cd21..ac1da7ade355d 100644 --- a/pandas/core/tools/timedeltas.py +++ b/pandas/core/tools/timedeltas.py @@ -143,7 +143,7 @@ def _coerce_scalar_to_timedelta_type(r, unit='ns', box=True, errors='raise'): """Convert string 'r' to a timedelta object.""" try: - result = parse_timedelta_string(r) + result = parse_timedelta_string(r, unit) result = np.timedelta64(result) except Exception: try: diff --git a/pandas/tests/scalar/timedelta/test_construction.py b/pandas/tests/scalar/timedelta/test_construction.py index 439ed1a16313a..9bb02a18c04a7 100644 --- a/pandas/tests/scalar/timedelta/test_construction.py +++ b/pandas/tests/scalar/timedelta/test_construction.py @@ -228,5 +228,5 @@ def test_string_with_unit(str_unit, unit, expectation): 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]) + assert all(pd.to_timedelta([val_str, val_str], unit=unit) == \ + pd.to_timedelta([expected_td, expected_td])) From 9bff5047a86041529deb076bd9dd454f6331206c Mon Sep 17 00:00:00 2001 From: Victor Date: Mon, 8 Oct 2018 18:05:30 +0200 Subject: [PATCH 16/38] Removed litle whitespace. --- doc/source/whatsnew/v0.24.0.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/source/whatsnew/v0.24.0.txt b/doc/source/whatsnew/v0.24.0.txt index 681b28b4b29af..cb8d812f2e781 100644 --- a/doc/source/whatsnew/v0.24.0.txt +++ b/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`) +- 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 ^^^^^^^^^ From b6c7d45f65538920f30f021f281eb24488a1a531 Mon Sep 17 00:00:00 2001 From: Victor Date: Mon, 8 Oct 2018 18:05:49 +0200 Subject: [PATCH 17/38] Type specified unit as object. --- pandas/_libs/tslibs/timedeltas.pxd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/_libs/tslibs/timedeltas.pxd b/pandas/_libs/tslibs/timedeltas.pxd index 4541a0e070884..92527ab496860 100644 --- a/pandas/_libs/tslibs/timedeltas.pxd +++ b/pandas/_libs/tslibs/timedeltas.pxd @@ -3,7 +3,7 @@ from numpy cimport int64_t # Exposed for tslib, not intended for outside use. -cpdef parse_timedelta_string(object ts, specified_unit=*) +cpdef parse_timedelta_string(object ts, object 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) From 9ff6ade1e69c997dd82d19f8b4b5d4fc350bfde6 Mon Sep 17 00:00:00 2001 From: Victor Date: Mon, 8 Oct 2018 18:17:28 +0200 Subject: [PATCH 18/38] Removed newline. --- pandas/tests/scalar/timedelta/test_construction.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pandas/tests/scalar/timedelta/test_construction.py b/pandas/tests/scalar/timedelta/test_construction.py index 9bb02a18c04a7..64d17a049f9bb 100644 --- a/pandas/tests/scalar/timedelta/test_construction.py +++ b/pandas/tests/scalar/timedelta/test_construction.py @@ -88,7 +88,6 @@ def test_construction(): # no units specified with pytest.raises(ValueError): Timedelta('3.1415') - with pytest.raises(ValueError): Timedelta('2000') @@ -217,8 +216,8 @@ def test_td_constructor_value_error(): @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)), + ("s", "d", pytest.raises(ValueError)), # Units doubly defined + ("s", "s", pytest.raises(ValueError)), # Units doubly defined (same) ("", None, pytest.raises(ValueError)), # No units ]) def test_string_with_unit(str_unit, unit, expectation): From be93e4be4015ab78740688847b7160e425a24c06 Mon Sep 17 00:00:00 2001 From: victor Date: Thu, 11 Oct 2018 00:45:50 +0200 Subject: [PATCH 19/38] Moved default unit down to more core functions. --- pandas/_libs/tslibs/timedeltas.pyx | 4 +++- pandas/core/tools/timedeltas.py | 10 ++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/pandas/_libs/tslibs/timedeltas.pyx b/pandas/_libs/tslibs/timedeltas.pyx index 29566b9118790..25f41853441a8 100644 --- a/pandas/_libs/tslibs/timedeltas.pyx +++ b/pandas/_libs/tslibs/timedeltas.pyx @@ -151,6 +151,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): @@ -198,7 +200,7 @@ cpdef convert_to_timedelta64(object ts, object unit): return ts.astype('timedelta64[ns]') -cpdef array_to_timedelta64(object[:] values, unit='ns', errors='raise'): +cpdef 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. diff --git a/pandas/core/tools/timedeltas.py b/pandas/core/tools/timedeltas.py index ac1da7ade355d..a153b05554fbd 100644 --- a/pandas/core/tools/timedeltas.py +++ b/pandas/core/tools/timedeltas.py @@ -17,7 +17,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 @@ -134,12 +134,12 @@ def _validate_timedelta_unit(arg): return _unit_map[arg] except (KeyError, TypeError): if arg is None: - return 'ns' + return None raise ValueError("invalid timedelta unit {arg} provided" .format(arg=arg)) -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: @@ -162,7 +162,7 @@ def _coerce_scalar_to_timedelta_type(r, unit='ns', box=True, errors='raise'): 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'): @@ -172,6 +172,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: From 85f010334844549c70109ca7f5d9c5ef497ab820 Mon Sep 17 00:00:00 2001 From: victor Date: Thu, 11 Oct 2018 00:53:01 +0200 Subject: [PATCH 20/38] Fixed linting. --- pandas/tests/indexes/timedeltas/test_partial_slicing.py | 6 ++++-- pandas/tests/scalar/timedelta/test_construction.py | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/pandas/tests/indexes/timedeltas/test_partial_slicing.py b/pandas/tests/indexes/timedeltas/test_partial_slicing.py index da8baa43cbb54..a79a3c15902c9 100644 --- a/pandas/tests/indexes/timedeltas/test_partial_slicing.py +++ b/pandas/tests/indexes/timedeltas/test_partial_slicing.py @@ -53,7 +53,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('0ns', 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): @@ -78,7 +79,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('0ns', 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', diff --git a/pandas/tests/scalar/timedelta/test_construction.py b/pandas/tests/scalar/timedelta/test_construction.py index 64d17a049f9bb..999d77f5a8280 100644 --- a/pandas/tests/scalar/timedelta/test_construction.py +++ b/pandas/tests/scalar/timedelta/test_construction.py @@ -227,5 +227,5 @@ def test_string_with_unit(str_unit, unit, expectation): 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])) + assert all(pd.to_timedelta([val_str, val_str], unit=unit) == + pd.to_timedelta([expected_td, expected_td])) From bada4a7630592f3911bd29a8f6877a7c0f36f9e1 Mon Sep 17 00:00:00 2001 From: Victor Maryama Date: Mon, 5 Nov 2018 13:50:59 +0100 Subject: [PATCH 21/38] Minor typo. --- doc/source/whatsnew/v0.24.0.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/source/whatsnew/v0.24.0.txt b/doc/source/whatsnew/v0.24.0.txt index 71887d71b6e34..352d9c774a313 100644 --- a/doc/source/whatsnew/v0.24.0.txt +++ b/doc/source/whatsnew/v0.24.0.txt @@ -1071,7 +1071,7 @@ 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` where passing a string of a pure number would not take unit into account. Also raises for ambiguous/duplicate unit specification (:issue:`12136`) Timezones ^^^^^^^^^ From 8c0425ae45a5134dbfc936faa5505b03d3466bb4 Mon Sep 17 00:00:00 2001 From: Victor Maryama Date: Mon, 5 Nov 2018 13:51:41 +0100 Subject: [PATCH 22/38] Lint --- pandas/_libs/tslibs/timedeltas.pyx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/_libs/tslibs/timedeltas.pyx b/pandas/_libs/tslibs/timedeltas.pyx index d88816ca16435..283ef325ad59e 100644 --- a/pandas/_libs/tslibs/timedeltas.pyx +++ b/pandas/_libs/tslibs/timedeltas.pyx @@ -152,7 +152,7 @@ cpdef convert_to_timedelta64(object ts, object unit): # handle the numpy < 1.7 case """ if unit is None: - unit='ns' + unit = 'ns' if checknull_with_nat(ts): return np.timedelta64(NPY_NAT) elif isinstance(ts, Timedelta): From 493a14b41caebbb20b2bcc18e634e1675f6dd6ea Mon Sep 17 00:00:00 2001 From: Victor Maryama Date: Mon, 5 Nov 2018 13:55:34 +0100 Subject: [PATCH 23/38] Using ValueError instead of Exception. --- pandas/core/tools/timedeltas.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/core/tools/timedeltas.py b/pandas/core/tools/timedeltas.py index a153b05554fbd..8234957a2da0c 100644 --- a/pandas/core/tools/timedeltas.py +++ b/pandas/core/tools/timedeltas.py @@ -145,7 +145,7 @@ def _coerce_scalar_to_timedelta_type(r, unit=None, box=True, errors='raise'): try: result = parse_timedelta_string(r, unit) result = np.timedelta64(result) - except Exception: + except ValueError: try: result = convert_to_timedelta64(r, unit) except ValueError: From 448b68a30f49ced9c0295118b46169353dc6b33b Mon Sep 17 00:00:00 2001 From: Victor Maryama Date: Mon, 5 Nov 2018 14:41:20 +0100 Subject: [PATCH 24/38] Catching TypeError as well. --- pandas/core/tools/timedeltas.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/core/tools/timedeltas.py b/pandas/core/tools/timedeltas.py index 8234957a2da0c..1b596f577968e 100644 --- a/pandas/core/tools/timedeltas.py +++ b/pandas/core/tools/timedeltas.py @@ -145,7 +145,7 @@ def _coerce_scalar_to_timedelta_type(r, unit=None, box=True, errors='raise'): try: result = parse_timedelta_string(r, unit) result = np.timedelta64(result) - except ValueError: + except (ValueError, TypeError): try: result = convert_to_timedelta64(r, unit) except ValueError: From bd173a9c478f371f093dc56bce8b905262e7adc2 Mon Sep 17 00:00:00 2001 From: Victor Maryama Date: Mon, 5 Nov 2018 16:26:06 +0100 Subject: [PATCH 25/38] LINT --- pandas/util/testing.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pandas/util/testing.py b/pandas/util/testing.py index 149b043b262f2..2cf68085ff13e 100644 --- a/pandas/util/testing.py +++ b/pandas/util/testing.py @@ -40,6 +40,7 @@ from pandas.io.common import urlopen from pandas.io.formats.printing import pprint_thing + class NullContextManager(object): def __init__(self, dummy_resource=None): self.dummy_resource = dummy_resource From 1cfc7296095c42c469e8b8457e589db02c459136 Mon Sep 17 00:00:00 2001 From: Victor Maryama Date: Tue, 6 Nov 2018 18:15:05 +0100 Subject: [PATCH 26/38] Refactored tests, fixed little bug in array_to_timedelta64 and explained NullContextManager. --- pandas/_libs/tslibs/timedeltas.pyx | 4 +-- .../scalar/timedelta/test_construction.py | 31 ++++++++++++------- pandas/util/testing.py | 21 +++++++++++++ 3 files changed, 42 insertions(+), 14 deletions(-) diff --git a/pandas/_libs/tslibs/timedeltas.pyx b/pandas/_libs/tslibs/timedeltas.pyx index 87231843d9451..d40d0ecbcb6f6 100644 --- a/pandas/_libs/tslibs/timedeltas.pyx +++ b/pandas/_libs/tslibs/timedeltas.pyx @@ -216,7 +216,7 @@ cpdef convert_to_timedelta64(object ts, object unit): @cython.boundscheck(False) @cython.wraparound(False) -def array_to_timedelta64(object[:] values, unit='None', 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. @@ -435,7 +435,7 @@ cpdef inline parse_timedelta_string(object ts, specified_unit=None): unit = [specified_unit] else: if specified_unit: - raise ValueError("units doubly specified") + 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 diff --git a/pandas/tests/scalar/timedelta/test_construction.py b/pandas/tests/scalar/timedelta/test_construction.py index 999d77f5a8280..151003b2806e4 100644 --- a/pandas/tests/scalar/timedelta/test_construction.py +++ b/pandas/tests/scalar/timedelta/test_construction.py @@ -85,12 +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') - with pytest.raises(ValueError): - Timedelta('2000') - # invalid construction tm.assert_raises_regex(ValueError, "cannot construct a Timedelta", lambda: Timedelta()) @@ -214,15 +208,28 @@ def test_td_constructor_value_error(): 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", [ - ("", "s", tm.do_not_raise), # Expected case - ("s", "d", pytest.raises(ValueError)), # Units doubly defined - ("s", "s", pytest.raises(ValueError)), # Units doubly defined (same) - ("", None, pytest.raises(ValueError)), # No units + # 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(str_unit, unit, expectation): +def test_string_with_unit(value, str_unit, unit, expectation): with expectation: - val_str = "10{}".format(str_unit) + val_str = "{}{}".format(value, str_unit) expected_td = Timedelta(10, unit=unit) assert Timedelta(val_str, unit=unit) == expected_td diff --git a/pandas/util/testing.py b/pandas/util/testing.py index 2cf68085ff13e..e3346b718bc3d 100644 --- a/pandas/util/testing.py +++ b/pandas/util/testing.py @@ -42,6 +42,27 @@ 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 From e559f06cf0aadf0045eeb7bae38d05b6107b6f8a Mon Sep 17 00:00:00 2001 From: victor Date: Tue, 6 Nov 2018 23:49:02 +0100 Subject: [PATCH 27/38] Fixed unit as None (instead of ns), Fixed test. --- pandas/_libs/tslibs/timedeltas.pyx | 5 ++--- pandas/tests/scalar/timedelta/test_construction.py | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/pandas/_libs/tslibs/timedeltas.pyx b/pandas/_libs/tslibs/timedeltas.pyx index d40d0ecbcb6f6..87f0c7ab7ea27 100644 --- a/pandas/_libs/tslibs/timedeltas.pyx +++ b/pandas/_libs/tslibs/timedeltas.pyx @@ -532,10 +532,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): diff --git a/pandas/tests/scalar/timedelta/test_construction.py b/pandas/tests/scalar/timedelta/test_construction.py index 151003b2806e4..db765827bb582 100644 --- a/pandas/tests/scalar/timedelta/test_construction.py +++ b/pandas/tests/scalar/timedelta/test_construction.py @@ -230,7 +230,7 @@ def test_td_constructor_value_error(): def test_string_with_unit(value, str_unit, unit, expectation): with expectation: val_str = "{}{}".format(value, str_unit) - expected_td = Timedelta(10, unit=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 From b91495047cc9f2171fb03df183f029c143243bb6 Mon Sep 17 00:00:00 2001 From: Victor Maryama Date: Sat, 10 Nov 2018 14:00:38 +0100 Subject: [PATCH 28/38] LINT: Fixed long lines. --- .../scalar/timedelta/test_construction.py | 24 ++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/pandas/tests/scalar/timedelta/test_construction.py b/pandas/tests/scalar/timedelta/test_construction.py index db765827bb582..dd672f61057bd 100644 --- a/pandas/tests/scalar/timedelta/test_construction.py +++ b/pandas/tests/scalar/timedelta/test_construction.py @@ -214,18 +214,30 @@ def test_td_constructor_value_error(): ]) @pytest.mark.parametrize("str_unit, unit, expectation", [ # Expected case - ("", "s", tm.do_not_raise), + ("", + "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)")), + ("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)")), + ("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")), + ("", + None, + pytest.raises(ValueError, + message="number string without units")), ]) def test_string_with_unit(value, str_unit, unit, expectation): with expectation: From 64a4440e97ed27016df6541c94093c5f4ee19477 Mon Sep 17 00:00:00 2001 From: Victor Maryama Date: Sat, 10 Nov 2018 15:05:23 +0100 Subject: [PATCH 29/38] LINT: Fixed long lines. --- pandas/_libs/tslibs/timedeltas.pyx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pandas/_libs/tslibs/timedeltas.pyx b/pandas/_libs/tslibs/timedeltas.pyx index 87f0c7ab7ea27..a345a78812589 100644 --- a/pandas/_libs/tslibs/timedeltas.pyx +++ b/pandas/_libs/tslibs/timedeltas.pyx @@ -435,7 +435,10 @@ cpdef inline parse_timedelta_string(object ts, specified_unit=None): unit = [specified_unit] else: if specified_unit: - raise ValueError("units were doubly specified, both as an argument ({}) and inside string ({}).".format(specified_unit, 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 From 4d264743d8da8fb71678f36109d8ebfd8c473c59 Mon Sep 17 00:00:00 2001 From: Victor Maryama Date: Sat, 10 Nov 2018 15:41:13 +0100 Subject: [PATCH 30/38] LINT: Fixed indentation of closing bracket. --- pandas/_libs/tslibs/timedeltas.pyx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/_libs/tslibs/timedeltas.pyx b/pandas/_libs/tslibs/timedeltas.pyx index a345a78812589..3a15903eae816 100644 --- a/pandas/_libs/tslibs/timedeltas.pyx +++ b/pandas/_libs/tslibs/timedeltas.pyx @@ -438,7 +438,7 @@ cpdef inline parse_timedelta_string(object ts, specified_unit=None): 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 From 2f5300461a2c87957a0f02547981da89a51c7349 Mon Sep 17 00:00:00 2001 From: victor Date: Mon, 26 Nov 2018 23:18:09 +0100 Subject: [PATCH 31/38] Added comment. --- pandas/_libs/tslibs/timedeltas.pyx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pandas/_libs/tslibs/timedeltas.pyx b/pandas/_libs/tslibs/timedeltas.pyx index 3a15903eae816..b1c494e81b5c4 100644 --- a/pandas/_libs/tslibs/timedeltas.pyx +++ b/pandas/_libs/tslibs/timedeltas.pyx @@ -535,6 +535,8 @@ cpdef inline object parse_timedelta_unit(object unit): ---------- unit : an unit string """ + + # Preserve, will be dealt with eventually in other functions if unit is None or unit == 'M': return unit From f5e59eed89cbc0c4f1abe45480086eddaa3c2446 Mon Sep 17 00:00:00 2001 From: victor Date: Tue, 27 Nov 2018 00:22:28 +0100 Subject: [PATCH 32/38] Casting unit to 'ns' if None. --- pandas/core/arrays/timedeltas.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pandas/core/arrays/timedeltas.py b/pandas/core/arrays/timedeltas.py index d1e6d979b554c..3fd0b396e90ac 100644 --- a/pandas/core/arrays/timedeltas.py +++ b/pandas/core/arrays/timedeltas.py @@ -521,6 +521,7 @@ def sequence_to_td64ns(data, copy=False, unit="ns", errors="raise"): # treat as multiples of the given unit. If after converting to nanos, # there are fractional components left, these are truncated # (i.e. NOT rounded) + unit = unit if unit is not None else "ns" mask = np.isnan(data) coeff = np.timedelta64(1, unit) / np.timedelta64(1, 'ns') data = (coeff * data).astype(np.int64).view('timedelta64[ns]') From e7266da4b7d103e06e8c80587e1c0e955cc0a80c Mon Sep 17 00:00:00 2001 From: victor Date: Tue, 27 Nov 2018 00:38:01 +0100 Subject: [PATCH 33/38] Updated documentation and issuing DeprecationWarning instead of exception. --- doc/source/whatsnew/v0.24.0.rst | 2 +- pandas/_libs/tslibs/timedeltas.pyx | 4 +++- pandas/tests/scalar/timedelta/test_construction.py | 5 +++-- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/doc/source/whatsnew/v0.24.0.rst b/doc/source/whatsnew/v0.24.0.rst index e475ddc6e1719..175773b7c656d 100644 --- a/doc/source/whatsnew/v0.24.0.rst +++ b/doc/source/whatsnew/v0.24.0.rst @@ -1224,7 +1224,7 @@ 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`) where passing a string of a pure number would not take unit into account. Also raises for ambiguous/duplicate unit specification and a number string without defined unit is deprecated (:issue:`12136`) - Bug in :class:`Timedelta` and :func:`to_timedelta()` have inconsistencies in supported unit string (:issue:`21762`) Timezones diff --git a/pandas/_libs/tslibs/timedeltas.pyx b/pandas/_libs/tslibs/timedeltas.pyx index b8fd1078ec710..800de9bffb058 100644 --- a/pandas/_libs/tslibs/timedeltas.pyx +++ b/pandas/_libs/tslibs/timedeltas.pyx @@ -486,7 +486,9 @@ cpdef inline parse_timedelta_string(object ts, specified_unit=None): if have_value: raise ValueError("have leftover units") if len(number): - raise ValueError("number string without units") + warnings.warn("number string without units is deprecated and + " will raise an exception in future versions. Considering as nanoseconds.") + result = timedelta_from_spec(number, frac, 'ns') return result diff --git a/pandas/tests/scalar/timedelta/test_construction.py b/pandas/tests/scalar/timedelta/test_construction.py index 643c0151b785e..25daea712eebb 100644 --- a/pandas/tests/scalar/timedelta/test_construction.py +++ b/pandas/tests/scalar/timedelta/test_construction.py @@ -234,8 +234,9 @@ def test_td_constructor_value_error(): # No units ("", None, - pytest.raises(ValueError, - message="number string without units")), + pytest.warns(DeprecationWarning, + message="number string without units is deprecated and " + " will raise an exception in future versions. Considering as nanoseconds.")), ]) def test_string_with_unit(value, str_unit, unit, expectation): with expectation: From b6a02dffd3de7d20214dded6095094eace32eb9a Mon Sep 17 00:00:00 2001 From: victor Date: Tue, 27 Nov 2018 00:43:55 +0100 Subject: [PATCH 34/38] Fixed linting. --- pandas/_libs/tslibs/timedeltas.pyx | 7 +++++-- .../indexes/timedeltas/test_partial_slicing.py | 3 ++- pandas/tests/scalar/timedelta/test_construction.py | 14 ++++++++------ 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/pandas/_libs/tslibs/timedeltas.pyx b/pandas/_libs/tslibs/timedeltas.pyx index 800de9bffb058..8da7cf27c4dfa 100644 --- a/pandas/_libs/tslibs/timedeltas.pyx +++ b/pandas/_libs/tslibs/timedeltas.pyx @@ -486,8 +486,11 @@ cpdef inline parse_timedelta_string(object ts, specified_unit=None): if have_value: raise ValueError("have leftover units") if len(number): - warnings.warn("number string without units is deprecated and - " will raise an exception in future versions. Considering as nanoseconds.") + warnings.warn( + "number string without units is deprecated and + " will raise an exception in future versions. Considering as nanoseconds.", + DeprecationWarning + ) result = timedelta_from_spec(number, frac, 'ns') return result diff --git a/pandas/tests/indexes/timedeltas/test_partial_slicing.py b/pandas/tests/indexes/timedeltas/test_partial_slicing.py index 29446a7a67987..2134a1c9034a5 100644 --- a/pandas/tests/indexes/timedeltas/test_partial_slicing.py +++ b/pandas/tests/indexes/timedeltas/test_partial_slicing.py @@ -77,7 +77,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('0ns', periods=20, freq='H')) + ts = Series(np.arange(20), + timedelta_range('0ns', periods=20, freq='H')) with pytest.raises(ValueError, match='slice step cannot be zero'): ts[::0] with pytest.raises(ValueError, match='slice step cannot be zero'): diff --git a/pandas/tests/scalar/timedelta/test_construction.py b/pandas/tests/scalar/timedelta/test_construction.py index 25daea712eebb..71e17344f02de 100644 --- a/pandas/tests/scalar/timedelta/test_construction.py +++ b/pandas/tests/scalar/timedelta/test_construction.py @@ -5,6 +5,7 @@ import pytest from pandas import Timedelta, offsets, to_timedelta +from pandas.util.testing import do_not_raise def test_construction(): @@ -214,7 +215,7 @@ def test_td_constructor_value_error(): # Expected case ("", "s", - tm.do_not_raise), + do_not_raise), # Units doubly defined ("s", @@ -235,8 +236,9 @@ def test_td_constructor_value_error(): ("", None, pytest.warns(DeprecationWarning, - message="number string without units is deprecated and " - " will raise an exception in future versions. Considering as nanoseconds.")), + message="number string without units is deprecated and " + " will raise an exception in future versions. " + "Considering as nanoseconds.")), ]) def test_string_with_unit(value, str_unit, unit, expectation): with expectation: @@ -244,6 +246,6 @@ def test_string_with_unit(value, str_unit, unit, expectation): 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])) + assert to_timedelta(val_str, unit=unit) == expected_td + assert all(to_timedelta([val_str, val_str], unit=unit) == + to_timedelta([expected_td, expected_td])) From 694abe9535886127596eb1975bf5324b9338b860 Mon Sep 17 00:00:00 2001 From: Victor Maryama Date: Tue, 27 Nov 2018 18:55:20 +0100 Subject: [PATCH 35/38] DOC: Reorganized whatsnew --- doc/source/whatsnew/v0.24.0.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/source/whatsnew/v0.24.0.rst b/doc/source/whatsnew/v0.24.0.rst index 175773b7c656d..b48f0a1fc9fd4 100644 --- a/doc/source/whatsnew/v0.24.0.rst +++ b/doc/source/whatsnew/v0.24.0.rst @@ -1046,6 +1046,7 @@ Deprecations - The ``keep_tz=False`` option (the default) of the ``keep_tz`` keyword of :meth:`DatetimeIndex.to_series` is deprecated (:issue:`17832`). - Timezone converting a tz-aware ``datetime.datetime`` or :class:`Timestamp` with :class:`Timestamp` and the ``tz`` argument is now deprecated. Instead, use :meth:`Timestamp.tz_convert` (:issue:`23579`) +- A timedelta passed a number string without a defined unit is deprecated (:issue:`12136`) .. _whatsnew_0240.deprecations.datetimelike_int_ops: @@ -1224,7 +1225,7 @@ 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` (and :func: `to_timedelta`) where passing a string of a pure number would not take unit into account. Also raises for ambiguous/duplicate unit specification and a number string without defined unit is deprecated (:issue:`12136`) +- Bug in :class:`Timedelta` (and :func: `to_timedelta`) where passing a string of a pure number would not take the unit into account. Now raises for an ambiguous or duplicate unit specification.(:issue:`12136`) - Bug in :class:`Timedelta` and :func:`to_timedelta()` have inconsistencies in supported unit string (:issue:`21762`) Timezones From 4d86ae12510d0289e23465399276b291de60a948 Mon Sep 17 00:00:00 2001 From: Victor Maryama Date: Tue, 27 Nov 2018 18:57:05 +0100 Subject: [PATCH 36/38] CLN: Minor refactoring. --- pandas/_libs/tslibs/timedeltas.pyx | 7 ++++--- pandas/core/arrays/timedeltas.py | 3 ++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/pandas/_libs/tslibs/timedeltas.pyx b/pandas/_libs/tslibs/timedeltas.pyx index 8da7cf27c4dfa..78e20d9afc485 100644 --- a/pandas/_libs/tslibs/timedeltas.pyx +++ b/pandas/_libs/tslibs/timedeltas.pyx @@ -148,7 +148,7 @@ cpdef int64_t delta_to_nanoseconds(delta) except? -1: raise TypeError(type(delta)) -cpdef convert_to_timedelta64(object ts, object unit): +cpdef convert_to_timedelta64(object ts, object unit=None): """ Convert an incoming object to a timedelta64 if possible. Before calling, unit must be standardized to avoid repeated unit conversion @@ -489,7 +489,7 @@ cpdef inline parse_timedelta_string(object ts, specified_unit=None): warnings.warn( "number string without units is deprecated and " will raise an exception in future versions. Considering as nanoseconds.", - DeprecationWarning + FutureWarning ) result = timedelta_from_spec(number, frac, 'ns') @@ -541,7 +541,8 @@ cpdef inline object parse_timedelta_unit(object unit): unit : an unit string """ - # Preserve, will be dealt with eventually in other functions + # Preserve unit if None, will be cast to nanoseconds + # later on at the proper functions if unit is None or unit == 'M': return unit diff --git a/pandas/core/arrays/timedeltas.py b/pandas/core/arrays/timedeltas.py index 3fd0b396e90ac..6e162f8e0718b 100644 --- a/pandas/core/arrays/timedeltas.py +++ b/pandas/core/arrays/timedeltas.py @@ -521,7 +521,8 @@ def sequence_to_td64ns(data, copy=False, unit="ns", errors="raise"): # treat as multiples of the given unit. If after converting to nanos, # there are fractional components left, these are truncated # (i.e. NOT rounded) - unit = unit if unit is not None else "ns" + if unit is None: + unit = "ns" mask = np.isnan(data) coeff = np.timedelta64(1, unit) / np.timedelta64(1, 'ns') data = (coeff * data).astype(np.int64).view('timedelta64[ns]') From 80a1161d36fb0da4a5d5489d1cbdcd78af2542ba Mon Sep 17 00:00:00 2001 From: Victor Maryama Date: Tue, 8 Jan 2019 16:14:09 +0100 Subject: [PATCH 37/38] Added default value to func declaration. --- pandas/_libs/tslibs/timedeltas.pxd | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/_libs/tslibs/timedeltas.pxd b/pandas/_libs/tslibs/timedeltas.pxd index 387525b5d71f5..b5732ee670ee8 100644 --- a/pandas/_libs/tslibs/timedeltas.pxd +++ b/pandas/_libs/tslibs/timedeltas.pxd @@ -6,4 +6,4 @@ from numpy cimport int64_t 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) +cpdef convert_to_timedelta64(object ts, object unit=*) From dbe8faec85da5c5ec858b30d9e464edb970e5cf0 Mon Sep 17 00:00:00 2001 From: Victor Maryama Date: Tue, 8 Jan 2019 16:23:32 +0100 Subject: [PATCH 38/38] Fixed bug in warning string. --- pandas/_libs/tslibs/timedeltas.pyx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pandas/_libs/tslibs/timedeltas.pyx b/pandas/_libs/tslibs/timedeltas.pyx index 1423821cca660..828ea22d7bbf1 100644 --- a/pandas/_libs/tslibs/timedeltas.pyx +++ b/pandas/_libs/tslibs/timedeltas.pyx @@ -484,7 +484,7 @@ cpdef inline parse_timedelta_string(object ts, specified_unit=None): raise ValueError("have leftover units") if len(number): warnings.warn( - "number string without units is deprecated and + "number string without units is deprecated and" " will raise an exception in future versions. Considering as nanoseconds.", FutureWarning )