From 41c9adceb8cfb2a0d286635b90f87c4b64274271 Mon Sep 17 00:00:00 2001 From: Con Date: Sat, 27 May 2023 18:43:04 -0400 Subject: [PATCH 01/12] adds raise for numpy Y and M conversion in Timedelta --- pandas/_libs/tslibs/timedeltas.pyx | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/pandas/_libs/tslibs/timedeltas.pyx b/pandas/_libs/tslibs/timedeltas.pyx index 047b5e861da2c..4610175dd24c6 100644 --- a/pandas/_libs/tslibs/timedeltas.pyx +++ b/pandas/_libs/tslibs/timedeltas.pyx @@ -1786,6 +1786,12 @@ class Timedelta(_Timedelta): return NaT reso = get_datetime64_unit(value) + if reso in [NPY_DATETIMEUNIT.NPY_FR_Y, NPY_DATETIMEUNIT.NPY_FR_M]: + raise ValueError( + "ValueError: Numpy timedelta64 Units 'Y' and 'M' are no longer \ + supported, as they do not represent unambiguous timedelta \ + values durations." + ) new_reso = get_supported_reso(reso) if reso != NPY_DATETIMEUNIT.NPY_FR_GENERIC: try: From 90d34f5e6050bde57193c75eb4abab07a5aebf80 Mon Sep 17 00:00:00 2001 From: Con Date: Mon, 29 May 2023 14:47:50 -0400 Subject: [PATCH 02/12] fix failing tests --- pandas/tests/libs/test_lib.py | 2 +- pandas/tests/scalar/timedelta/test_constructors.py | 2 -- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/pandas/tests/libs/test_lib.py b/pandas/tests/libs/test_lib.py index 383e1b81e17a7..72de550ff36a6 100644 --- a/pandas/tests/libs/test_lib.py +++ b/pandas/tests/libs/test_lib.py @@ -58,7 +58,7 @@ def test_fast_multiget_timedelta_resos(self): tm.assert_numpy_array_equal(result, expected) # case that can't be cast to td64ns - td = Timedelta(np.timedelta64(400, "Y")) + td = Timedelta(np.timedelta64(146000, "D")) assert hash(td) == hash(td.as_unit("ms")) assert hash(td) == hash(td.as_unit("us")) mapping1 = {td: 1} diff --git a/pandas/tests/scalar/timedelta/test_constructors.py b/pandas/tests/scalar/timedelta/test_constructors.py index ad9dd408fbeaf..7bd9e5fc5e293 100644 --- a/pandas/tests/scalar/timedelta/test_constructors.py +++ b/pandas/tests/scalar/timedelta/test_constructors.py @@ -289,7 +289,6 @@ def test_overflow_on_construction(): @pytest.mark.parametrize( "val, unit", [ - (3508, "M"), (15251, "W"), # 1 (106752, "D"), # change from previous: (2562048, "h"), # 0 hours @@ -333,7 +332,6 @@ def test_construction_out_of_bounds_td64ns(val, unit): @pytest.mark.parametrize( "val, unit", [ - (3508 * 10**9, "M"), (15251 * 10**9, "W"), (106752 * 10**9, "D"), (2562048 * 10**9, "h"), From 3259eb9245b5e85c2dc117adba0f4b53b354863c Mon Sep 17 00:00:00 2001 From: Con Date: Tue, 30 May 2023 19:10:15 -0400 Subject: [PATCH 03/12] update raise lang --- pandas/_libs/tslibs/timedeltas.pyx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pandas/_libs/tslibs/timedeltas.pyx b/pandas/_libs/tslibs/timedeltas.pyx index 4610175dd24c6..06ff43d3a02f4 100644 --- a/pandas/_libs/tslibs/timedeltas.pyx +++ b/pandas/_libs/tslibs/timedeltas.pyx @@ -43,6 +43,7 @@ from pandas._libs.tslibs.conversion cimport ( from pandas._libs.tslibs.dtypes cimport ( get_supported_reso, npy_unit_to_abbrev, + npy_unit_to_attrname, ) from pandas._libs.tslibs.nattype cimport ( NPY_NAT, @@ -1788,9 +1789,7 @@ class Timedelta(_Timedelta): reso = get_datetime64_unit(value) if reso in [NPY_DATETIMEUNIT.NPY_FR_Y, NPY_DATETIMEUNIT.NPY_FR_M]: raise ValueError( - "ValueError: Numpy timedelta64 Units 'Y' and 'M' are no longer \ - supported, as they do not represent unambiguous timedelta \ - values durations." + f"{npy_unit_to_attrname[reso]} is not supported" ) new_reso = get_supported_reso(reso) if reso != NPY_DATETIMEUNIT.NPY_FR_GENERIC: From 20dcb1655bd1facb4c74c7d7a7997271e751861c Mon Sep 17 00:00:00 2001 From: Con Date: Mon, 5 Jun 2023 18:43:59 -0400 Subject: [PATCH 04/12] alternative approach --- pandas/_libs/tslibs/timedeltas.pyx | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/pandas/_libs/tslibs/timedeltas.pyx b/pandas/_libs/tslibs/timedeltas.pyx index 06ff43d3a02f4..dea503e7c32d5 100644 --- a/pandas/_libs/tslibs/timedeltas.pyx +++ b/pandas/_libs/tslibs/timedeltas.pyx @@ -43,7 +43,6 @@ from pandas._libs.tslibs.conversion cimport ( from pandas._libs.tslibs.dtypes cimport ( get_supported_reso, npy_unit_to_abbrev, - npy_unit_to_attrname, ) from pandas._libs.tslibs.nattype cimport ( NPY_NAT, @@ -1740,8 +1739,10 @@ class Timedelta(_Timedelta): + int(kwargs.get("milliseconds", 0) * 1_000_000) + seconds ) - - if unit in {"Y", "y", "M"}: + if unit in {"Y", "y", "M"} or (isinstance(value, np.timedelta64) and + get_datetime64_unit(value) in { + NPY_DATETIMEUNIT.NPY_FR_Y, + NPY_DATETIMEUNIT.NPY_FR_M}): raise ValueError( "Units 'M', 'Y', and 'y' are no longer supported, as they do not " "represent unambiguous timedelta values durations." @@ -1787,10 +1788,6 @@ class Timedelta(_Timedelta): return NaT reso = get_datetime64_unit(value) - if reso in [NPY_DATETIMEUNIT.NPY_FR_Y, NPY_DATETIMEUNIT.NPY_FR_M]: - raise ValueError( - f"{npy_unit_to_attrname[reso]} is not supported" - ) new_reso = get_supported_reso(reso) if reso != NPY_DATETIMEUNIT.NPY_FR_GENERIC: try: From cc48ef794b8abbed0f2f13d2810a13fca883bdd6 Mon Sep 17 00:00:00 2001 From: Con Date: Wed, 7 Jun 2023 19:39:40 -0400 Subject: [PATCH 05/12] add missing periods --- pandas/_libs/tslibs/timedeltas.pyx | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/pandas/_libs/tslibs/timedeltas.pyx b/pandas/_libs/tslibs/timedeltas.pyx index dea503e7c32d5..462653c72dfa2 100644 --- a/pandas/_libs/tslibs/timedeltas.pyx +++ b/pandas/_libs/tslibs/timedeltas.pyx @@ -44,6 +44,9 @@ from pandas._libs.tslibs.dtypes cimport ( get_supported_reso, npy_unit_to_abbrev, ) + +from pandas._libs.tslibs.dtypes import is_supported_unit + from pandas._libs.tslibs.nattype cimport ( NPY_NAT, c_NaT as NaT, @@ -1739,10 +1742,7 @@ class Timedelta(_Timedelta): + int(kwargs.get("milliseconds", 0) * 1_000_000) + seconds ) - if unit in {"Y", "y", "M"} or (isinstance(value, np.timedelta64) and - get_datetime64_unit(value) in { - NPY_DATETIMEUNIT.NPY_FR_Y, - NPY_DATETIMEUNIT.NPY_FR_M}): + if unit in {"Y", "y", "M"}: raise ValueError( "Units 'M', 'Y', and 'y' are no longer supported, as they do not " "represent unambiguous timedelta values durations." @@ -1788,6 +1788,15 @@ class Timedelta(_Timedelta): return NaT reso = get_datetime64_unit(value) + if not (is_supported_unit(reso) or + reso in [NPY_DATETIMEUNIT.NPY_FR_m, + NPY_DATETIMEUNIT.NPY_FR_h, + NPY_DATETIMEUNIT.NPY_FR_D, + NPY_DATETIMEUNIT.NPY_FR_W, + NPY_DATETIMEUNIT.NPY_FR_GENERIC]): + err = npy_unit_to_abbrev(reso) + raise ValueError(f" cannot construct a Timedelta from a unit {err}") + new_reso = get_supported_reso(reso) if reso != NPY_DATETIMEUNIT.NPY_FR_GENERIC: try: From 77a256c355dc4321f63737c57bb83f3dc19932da Mon Sep 17 00:00:00 2001 From: Con Date: Fri, 9 Jun 2023 14:00:35 -0400 Subject: [PATCH 06/12] switch to cimport --- pandas/_libs/tslibs/dtypes.pxd | 1 + pandas/_libs/tslibs/dtypes.pyx | 2 +- pandas/_libs/tslibs/timedeltas.pyx | 4 +--- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/pandas/_libs/tslibs/dtypes.pxd b/pandas/_libs/tslibs/dtypes.pxd index 1dca498f61b0e..a8dd88c763c14 100644 --- a/pandas/_libs/tslibs/dtypes.pxd +++ b/pandas/_libs/tslibs/dtypes.pxd @@ -9,6 +9,7 @@ cdef NPY_DATETIMEUNIT freq_group_code_to_npy_unit(int freq) noexcept nogil cpdef int64_t periods_per_day(NPY_DATETIMEUNIT reso=*) except? -1 cpdef int64_t periods_per_second(NPY_DATETIMEUNIT reso) except? -1 cpdef NPY_DATETIMEUNIT get_supported_reso(NPY_DATETIMEUNIT reso) +cpdef bint is_supported_unit(NPY_DATETIMEUNIT reso) cdef dict attrname_to_abbrevs cdef dict npy_unit_to_attrname diff --git a/pandas/_libs/tslibs/dtypes.pyx b/pandas/_libs/tslibs/dtypes.pyx index 2a49ea6760e72..19f4c83e6cecf 100644 --- a/pandas/_libs/tslibs/dtypes.pyx +++ b/pandas/_libs/tslibs/dtypes.pyx @@ -321,7 +321,7 @@ cpdef NPY_DATETIMEUNIT get_supported_reso(NPY_DATETIMEUNIT reso): return reso -def is_supported_unit(NPY_DATETIMEUNIT reso): +cpdef bint is_supported_unit(NPY_DATETIMEUNIT reso): return ( reso == NPY_DATETIMEUNIT.NPY_FR_ns or reso == NPY_DATETIMEUNIT.NPY_FR_us diff --git a/pandas/_libs/tslibs/timedeltas.pyx b/pandas/_libs/tslibs/timedeltas.pyx index 462653c72dfa2..f41c57eae702a 100644 --- a/pandas/_libs/tslibs/timedeltas.pyx +++ b/pandas/_libs/tslibs/timedeltas.pyx @@ -42,11 +42,9 @@ from pandas._libs.tslibs.conversion cimport ( ) from pandas._libs.tslibs.dtypes cimport ( get_supported_reso, + is_supported_unit, npy_unit_to_abbrev, ) - -from pandas._libs.tslibs.dtypes import is_supported_unit - from pandas._libs.tslibs.nattype cimport ( NPY_NAT, c_NaT as NaT, From 97be59c25ca4372540b87b8156b951f70d35a0d4 Mon Sep 17 00:00:00 2001 From: Con Date: Fri, 9 Jun 2023 14:30:24 -0400 Subject: [PATCH 07/12] add cython directive --- 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 f41c57eae702a..dac4744747e50 100644 --- a/pandas/_libs/tslibs/timedeltas.pyx +++ b/pandas/_libs/tslibs/timedeltas.pyx @@ -1,3 +1,5 @@ +# distutils: define_macros=NPY_NO_DEPRECATED_API=NPY_1_7_API_VERSION + import collections import warnings From 2ae5b726a893e7cc4a03a0585abe60e1de6da794 Mon Sep 17 00:00:00 2001 From: Con Date: Fri, 9 Jun 2023 14:52:13 -0400 Subject: [PATCH 08/12] switch to inline cython directive --- pandas/_libs/tslibs/timedeltas.pyx | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/pandas/_libs/tslibs/timedeltas.pyx b/pandas/_libs/tslibs/timedeltas.pyx index dac4744747e50..8c19f7c963283 100644 --- a/pandas/_libs/tslibs/timedeltas.pyx +++ b/pandas/_libs/tslibs/timedeltas.pyx @@ -1,4 +1,4 @@ -# distutils: define_macros=NPY_NO_DEPRECATED_API=NPY_1_7_API_VERSION + import collections import warnings @@ -155,9 +155,15 @@ cdef dict timedelta_abbrevs = { _no_input = object() +cdef extern from *: + """ + #define NPY_NO_DEPRECATED_API NPY_1_7_API_VERSION + """ + # ---------------------------------------------------------------------- # API + @cython.boundscheck(False) @cython.wraparound(False) def ints_to_pytimedelta(ndarray m8values, box=False): From 69e1f963f36fb1b01539039444b597df3dfbcbf3 Mon Sep 17 00:00:00 2001 From: Con Date: Fri, 9 Jun 2023 16:28:16 -0400 Subject: [PATCH 09/12] remove unsupported td operation --- doc/source/user_guide/timedeltas.rst | 3 --- pandas/_libs/tslibs/timedeltas.pyx | 6 ------ 2 files changed, 9 deletions(-) diff --git a/doc/source/user_guide/timedeltas.rst b/doc/source/user_guide/timedeltas.rst index 3a75aa0b39b1f..a6eb96f91a4bf 100644 --- a/doc/source/user_guide/timedeltas.rst +++ b/doc/source/user_guide/timedeltas.rst @@ -259,9 +259,6 @@ an alternative is to divide by another timedelta object. Note that division by t # to days td / np.timedelta64(1, "D") - # to months (these are constant months) - td / np.timedelta64(1, "M") - Dividing or multiplying a ``timedelta64[ns]`` Series by an integer or integer Series yields another ``timedelta64[ns]`` dtypes Series. diff --git a/pandas/_libs/tslibs/timedeltas.pyx b/pandas/_libs/tslibs/timedeltas.pyx index 8c19f7c963283..387cff9a5d140 100644 --- a/pandas/_libs/tslibs/timedeltas.pyx +++ b/pandas/_libs/tslibs/timedeltas.pyx @@ -154,12 +154,6 @@ cdef dict timedelta_abbrevs = { _no_input = object() - -cdef extern from *: - """ - #define NPY_NO_DEPRECATED_API NPY_1_7_API_VERSION - """ - # ---------------------------------------------------------------------- # API From 09fb208565791a8922bb518fcf338bdf6cfbce0d Mon Sep 17 00:00:00 2001 From: Con Date: Fri, 9 Jun 2023 16:43:20 -0400 Subject: [PATCH 10/12] adds whatnew update --- doc/source/whatsnew/v2.1.0.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/source/whatsnew/v2.1.0.rst b/doc/source/whatsnew/v2.1.0.rst index 762f41b4049c2..4711ff9d8826e 100644 --- a/doc/source/whatsnew/v2.1.0.rst +++ b/doc/source/whatsnew/v2.1.0.rst @@ -347,6 +347,7 @@ Timedelta ^^^^^^^^^ - :meth:`TimedeltaIndex.map` with ``na_action="ignore"`` now works as expected (:issue:`51644`) - Bug in :class:`TimedeltaIndex` division or multiplication leading to ``.freq`` of "0 Days" instead of ``None`` (:issue:`51575`) +- Bug in :class:`Timedelta` with Numpy timedelta64 objects not properly raising ``ValueError`` (:issue:`52806`) - Bug in :meth:`Timedelta.round` with values close to the implementation bounds returning incorrect results instead of raising ``OutOfBoundsTimedelta`` (:issue:`51494`) - Bug in :meth:`arrays.TimedeltaArray.map` and :meth:`TimedeltaIndex.map`, where the supplied callable operated array-wise instead of element-wise (:issue:`51977`) - From cc1ec83cfa55e34e121d573840732f90a495ea92 Mon Sep 17 00:00:00 2001 From: Con Date: Mon, 19 Jun 2023 10:31:19 -0400 Subject: [PATCH 11/12] remove whitespace --- pandas/_libs/tslibs/timedeltas.pyx | 2 -- 1 file changed, 2 deletions(-) diff --git a/pandas/_libs/tslibs/timedeltas.pyx b/pandas/_libs/tslibs/timedeltas.pyx index e24ca33208703..7319eccf3b51f 100644 --- a/pandas/_libs/tslibs/timedeltas.pyx +++ b/pandas/_libs/tslibs/timedeltas.pyx @@ -1,5 +1,3 @@ - - import collections import warnings From 3057fe8911f054b667a1ba6548cc7184ecbfc41f Mon Sep 17 00:00:00 2001 From: Con Date: Mon, 19 Jun 2023 11:02:56 -0400 Subject: [PATCH 12/12] adds test --- pandas/tests/tslibs/test_timedeltas.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pandas/tests/tslibs/test_timedeltas.py b/pandas/tests/tslibs/test_timedeltas.py index 36ca02d32dbbd..2308aa27b60ab 100644 --- a/pandas/tests/tslibs/test_timedeltas.py +++ b/pandas/tests/tslibs/test_timedeltas.py @@ -72,6 +72,15 @@ def test_delta_to_nanoseconds_td64_MY_raises(): delta_to_nanoseconds(td) +@pytest.mark.parametrize("unit", ["Y", "M"]) +def test_unsupported_td64_unit_raises(unit): + # GH 52806 + with pytest.raises( + ValueError, match=f"cannot construct a Timedelta from a unit {unit}" + ): + Timedelta(np.timedelta64(1, unit)) + + def test_huge_nanoseconds_overflow(): # GH 32402 assert delta_to_nanoseconds(Timedelta(1e10)) == 1e10