From f954d9bdcbaded3d664639d03e582fd8d284def7 Mon Sep 17 00:00:00 2001 From: Brock Date: Fri, 30 Sep 2022 14:01:46 -0700 Subject: [PATCH 1/4] BUG: Timedelta.__new__ --- doc/source/whatsnew/v1.6.0.rst | 2 ++ pandas/_libs/tslibs/timedeltas.pyx | 14 +++++++++++--- pandas/tests/scalar/timedelta/test_constructors.py | 14 ++++++++++++++ 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/doc/source/whatsnew/v1.6.0.rst b/doc/source/whatsnew/v1.6.0.rst index 5dd67458970aa..f9ab7056cfc0d 100644 --- a/doc/source/whatsnew/v1.6.0.rst +++ b/doc/source/whatsnew/v1.6.0.rst @@ -172,6 +172,8 @@ Datetimelike Timedelta ^^^^^^^^^ - Bug in :func:`to_timedelta` raising error when input has nullable dtype ``Float64`` (:issue:`48796`) +- Bug in :class:`Timedelta` constructor incorrectly raising instead of returning ``NaT`` when given a ``np.timedelta64("nat")`` (:issue:`?`) +- Bug in :class:`Timedelta` constructor failing to raise when passed both a :class:`Timedelta` object and keywords (e.g. days, seconds) (:issue:`?`) - Timezones diff --git a/pandas/_libs/tslibs/timedeltas.pyx b/pandas/_libs/tslibs/timedeltas.pyx index bf22967f615c4..99d768ce1f3d4 100644 --- a/pandas/_libs/tslibs/timedeltas.pyx +++ b/pandas/_libs/tslibs/timedeltas.pyx @@ -1662,10 +1662,15 @@ class Timedelta(_Timedelta): # GH 30543 if pd.Timedelta already passed, return it # check that only value is passed - if isinstance(value, _Timedelta) and unit is None and len(kwargs) == 0: + if isinstance(value, _Timedelta): + # 'unit' is benign in this case, but e.g. days or seconds + # doesn't make sense here. + if len(kwargs): + assert False + raise ValueError( + "Cannot pass both a Timedelta input and timedelta keyword arguments." + ) return value - elif isinstance(value, _Timedelta): - value = value.value elif isinstance(value, str): if unit is not None: raise ValueError("unit must not be specified if the value is a str") @@ -1679,6 +1684,9 @@ class Timedelta(_Timedelta): elif PyDelta_Check(value): value = convert_to_timedelta64(value, 'ns') elif is_timedelta64_object(value): + if get_timedelta64_value(value) == NPY_NAT: + # i.e. np.timedelta64("NaT") + return NaT value = ensure_td64ns(value) elif is_tick_object(value): value = np.timedelta64(value.nanos, 'ns') diff --git a/pandas/tests/scalar/timedelta/test_constructors.py b/pandas/tests/scalar/timedelta/test_constructors.py index 5b2438ec30f3a..c76d6f3279d06 100644 --- a/pandas/tests/scalar/timedelta/test_constructors.py +++ b/pandas/tests/scalar/timedelta/test_constructors.py @@ -7,6 +7,7 @@ from pandas._libs.tslibs import OutOfBoundsTimedelta from pandas import ( + NaT, Timedelta, offsets, to_timedelta, @@ -371,6 +372,14 @@ def test_timedelta_constructor_identity(): assert result is expected +def test_timedelta_pass_td_and_kwargs_raises(): + # don't silently ignore the kwargs + td = Timedelta(days=1) + msg = "Cannot pass both a Timedelta input and timedelta keyword arguments" + with pytest.raises(ValueError, match=msg): + Timedelta(td, days=2) + + @pytest.mark.parametrize( "constructor, value, unit, expectation", [ @@ -402,3 +411,8 @@ def test_string_without_numbers(value): ) with pytest.raises(ValueError, match=msg): Timedelta(value) + + +def test_timedelta_new_npnat(): + nat = np.timedelta64("NaT", "h") + assert Timedelta(nat) is NaT From ae63da8783694308b366cd61b7f2681d80b30174 Mon Sep 17 00:00:00 2001 From: Brock Date: Fri, 30 Sep 2022 14:02:53 -0700 Subject: [PATCH 2/4] remove assertion --- pandas/_libs/tslibs/timedeltas.pyx | 1 - 1 file changed, 1 deletion(-) diff --git a/pandas/_libs/tslibs/timedeltas.pyx b/pandas/_libs/tslibs/timedeltas.pyx index 99d768ce1f3d4..8d6c940db19ce 100644 --- a/pandas/_libs/tslibs/timedeltas.pyx +++ b/pandas/_libs/tslibs/timedeltas.pyx @@ -1666,7 +1666,6 @@ class Timedelta(_Timedelta): # 'unit' is benign in this case, but e.g. days or seconds # doesn't make sense here. if len(kwargs): - assert False raise ValueError( "Cannot pass both a Timedelta input and timedelta keyword arguments." ) From 71653f0b66427d70b3140bd3f3d37cdeb02767d5 Mon Sep 17 00:00:00 2001 From: Brock Date: Fri, 30 Sep 2022 14:04:23 -0700 Subject: [PATCH 3/4] GH refs --- doc/source/whatsnew/v1.6.0.rst | 4 ++-- pandas/_libs/tslibs/timedeltas.pyx | 1 + pandas/tests/scalar/timedelta/test_constructors.py | 3 ++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/doc/source/whatsnew/v1.6.0.rst b/doc/source/whatsnew/v1.6.0.rst index f9ab7056cfc0d..3851c975b70f1 100644 --- a/doc/source/whatsnew/v1.6.0.rst +++ b/doc/source/whatsnew/v1.6.0.rst @@ -172,8 +172,8 @@ Datetimelike Timedelta ^^^^^^^^^ - Bug in :func:`to_timedelta` raising error when input has nullable dtype ``Float64`` (:issue:`48796`) -- Bug in :class:`Timedelta` constructor incorrectly raising instead of returning ``NaT`` when given a ``np.timedelta64("nat")`` (:issue:`?`) -- Bug in :class:`Timedelta` constructor failing to raise when passed both a :class:`Timedelta` object and keywords (e.g. days, seconds) (:issue:`?`) +- Bug in :class:`Timedelta` constructor incorrectly raising instead of returning ``NaT`` when given a ``np.timedelta64("nat")`` (:issue:`48898`) +- Bug in :class:`Timedelta` constructor failing to raise when passed both a :class:`Timedelta` object and keywords (e.g. days, seconds) (:issue:`48898`) - Timezones diff --git a/pandas/_libs/tslibs/timedeltas.pyx b/pandas/_libs/tslibs/timedeltas.pyx index 8d6c940db19ce..a434a52356e54 100644 --- a/pandas/_libs/tslibs/timedeltas.pyx +++ b/pandas/_libs/tslibs/timedeltas.pyx @@ -1666,6 +1666,7 @@ class Timedelta(_Timedelta): # 'unit' is benign in this case, but e.g. days or seconds # doesn't make sense here. if len(kwargs): + # GH#48898 raise ValueError( "Cannot pass both a Timedelta input and timedelta keyword arguments." ) diff --git a/pandas/tests/scalar/timedelta/test_constructors.py b/pandas/tests/scalar/timedelta/test_constructors.py index c76d6f3279d06..25f75390b9d6c 100644 --- a/pandas/tests/scalar/timedelta/test_constructors.py +++ b/pandas/tests/scalar/timedelta/test_constructors.py @@ -373,7 +373,7 @@ def test_timedelta_constructor_identity(): def test_timedelta_pass_td_and_kwargs_raises(): - # don't silently ignore the kwargs + # don't silently ignore the kwargs GH#48898 td = Timedelta(days=1) msg = "Cannot pass both a Timedelta input and timedelta keyword arguments" with pytest.raises(ValueError, match=msg): @@ -414,5 +414,6 @@ def test_string_without_numbers(value): def test_timedelta_new_npnat(): + # GH#48898 nat = np.timedelta64("NaT", "h") assert Timedelta(nat) is NaT From b32221b40774f85a6a7a225a0fa07cd9821853ba Mon Sep 17 00:00:00 2001 From: Brock Date: Mon, 3 Oct 2022 12:41:15 -0700 Subject: [PATCH 4/4] put passed kwargs in exception message --- pandas/_libs/tslibs/timedeltas.pyx | 3 ++- pandas/tests/scalar/timedelta/test_constructors.py | 5 ++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/pandas/_libs/tslibs/timedeltas.pyx b/pandas/_libs/tslibs/timedeltas.pyx index a434a52356e54..545de31159930 100644 --- a/pandas/_libs/tslibs/timedeltas.pyx +++ b/pandas/_libs/tslibs/timedeltas.pyx @@ -1668,7 +1668,8 @@ class Timedelta(_Timedelta): if len(kwargs): # GH#48898 raise ValueError( - "Cannot pass both a Timedelta input and timedelta keyword arguments." + "Cannot pass both a Timedelta input and timedelta keyword arguments, got " + f"{list(kwargs.keys())}" ) return value elif isinstance(value, str): diff --git a/pandas/tests/scalar/timedelta/test_constructors.py b/pandas/tests/scalar/timedelta/test_constructors.py index 25f75390b9d6c..9e1cac4bd2627 100644 --- a/pandas/tests/scalar/timedelta/test_constructors.py +++ b/pandas/tests/scalar/timedelta/test_constructors.py @@ -375,7 +375,10 @@ def test_timedelta_constructor_identity(): def test_timedelta_pass_td_and_kwargs_raises(): # don't silently ignore the kwargs GH#48898 td = Timedelta(days=1) - msg = "Cannot pass both a Timedelta input and timedelta keyword arguments" + msg = ( + "Cannot pass both a Timedelta input and timedelta keyword arguments, " + r"got \['days'\]" + ) with pytest.raises(ValueError, match=msg): Timedelta(td, days=2)