From 48d9dfa68115907e696bd22e98eeda8a125d940c Mon Sep 17 00:00:00 2001 From: Marko Pacak Date: Thu, 1 Dec 2022 16:52:33 +0100 Subject: [PATCH 01/17] BUG: fix milliseconds and add test --- pandas/_libs/tslibs/offsets.pyx | 5 ++++- pandas/tests/tseries/offsets/test_offsets.py | 8 ++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/pandas/_libs/tslibs/offsets.pyx b/pandas/_libs/tslibs/offsets.pyx index 4c6493652b216..fbb467e8bfed8 100644 --- a/pandas/_libs/tslibs/offsets.pyx +++ b/pandas/_libs/tslibs/offsets.pyx @@ -315,6 +315,10 @@ cdef _determine_offset(kwds): use_relativedelta = False if len(kwds_no_nanos) > 0: if any(k in _kwds_use_relativedelta for k in kwds_no_nanos): + if "milliseconds" in kwds_no_nanos: + offset = timedelta(**kwds_no_nanos) + return offset, use_relativedelta + if "millisecond" in kwds_no_nanos: raise NotImplementedError( "Using DateOffset to replace `millisecond` component in " @@ -1163,7 +1167,6 @@ cdef class RelativeDeltaOffset(BaseOffset): def __init__(self, n=1, normalize=False, **kwds): BaseOffset.__init__(self, n, normalize) - off, use_rd = _determine_offset(kwds) object.__setattr__(self, "_offset", off) object.__setattr__(self, "_use_relativedelta", use_rd) diff --git a/pandas/tests/tseries/offsets/test_offsets.py b/pandas/tests/tseries/offsets/test_offsets.py index 63594c2b2c48a..84bdda29fbb4b 100644 --- a/pandas/tests/tseries/offsets/test_offsets.py +++ b/pandas/tests/tseries/offsets/test_offsets.py @@ -739,6 +739,14 @@ def test_eq(self): assert DateOffset(milliseconds=3) != DateOffset(milliseconds=7) + def test_milliseconds_combinations(self): + """ + Milliseconds in combination with another plural parameter should + not raise an error. + """ + # https://github.com/pytest-dev/pytest/issues/10525 + DateOffset(days=1, milliseconds=1) + class TestOffsetNames: def test_get_offset_name(self): From 65cd5da8ddaa56e2fe5c53e9379a442ede27fa4f Mon Sep 17 00:00:00 2001 From: Marko Pacak Date: Fri, 2 Dec 2022 11:54:02 +0100 Subject: [PATCH 02/17] BUG: move condition-check outside main if-block --- pandas/_libs/tslibs/offsets.pyx | 7 +++---- pandas/tests/tseries/offsets/test_offsets.py | 8 ++++---- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/pandas/_libs/tslibs/offsets.pyx b/pandas/_libs/tslibs/offsets.pyx index fbb467e8bfed8..51b70e824d40e 100644 --- a/pandas/_libs/tslibs/offsets.pyx +++ b/pandas/_libs/tslibs/offsets.pyx @@ -314,11 +314,10 @@ cdef _determine_offset(kwds): use_relativedelta = False if len(kwds_no_nanos) > 0: - if any(k in _kwds_use_relativedelta for k in kwds_no_nanos): - if "milliseconds" in kwds_no_nanos: - offset = timedelta(**kwds_no_nanos) - return offset, use_relativedelta + if "milliseconds" in kwds_no_nanos: + return timedelta(**kwds_no_nanos), use_relativedelta + if any(k in _kwds_use_relativedelta for k in kwds_no_nanos): if "millisecond" in kwds_no_nanos: raise NotImplementedError( "Using DateOffset to replace `millisecond` component in " diff --git a/pandas/tests/tseries/offsets/test_offsets.py b/pandas/tests/tseries/offsets/test_offsets.py index 84bdda29fbb4b..4a34d0bf6a516 100644 --- a/pandas/tests/tseries/offsets/test_offsets.py +++ b/pandas/tests/tseries/offsets/test_offsets.py @@ -739,12 +739,12 @@ def test_eq(self): assert DateOffset(milliseconds=3) != DateOffset(milliseconds=7) - def test_milliseconds_combinations(self): + def test_milliseconds_combination(self): """ - Milliseconds in combination with another plural parameter should - not raise an error. + The combination of 'milliseconds' (plural) and another parameter + should not raise an error. + See https://github.com/pytest-dev/pytest/issues/10525 """ - # https://github.com/pytest-dev/pytest/issues/10525 DateOffset(days=1, milliseconds=1) From f9896743bdfc2c44aaf276b302d55bed6a962c39 Mon Sep 17 00:00:00 2001 From: Marko Pacak Date: Sat, 3 Dec 2022 14:09:10 +0100 Subject: [PATCH 03/17] DOC: update whatsnew --- doc/source/whatsnew/v2.0.0.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/source/whatsnew/v2.0.0.rst b/doc/source/whatsnew/v2.0.0.rst index df82bcd37e971..4c2791b173f40 100644 --- a/doc/source/whatsnew/v2.0.0.rst +++ b/doc/source/whatsnew/v2.0.0.rst @@ -638,6 +638,7 @@ Datetimelike - Bug in subtracting a ``datetime`` scalar from :class:`DatetimeIndex` failing to retain the original ``freq`` attribute (:issue:`48818`) - Bug in ``pandas.tseries.holiday.Holiday`` where a half-open date interval causes inconsistent return types from :meth:`USFederalHolidayCalendar.holidays` (:issue:`49075`) - Bug in rendering :class:`DatetimeIndex` and :class:`Series` and :class:`DataFrame` with timezone-aware dtypes with ``dateutil`` or ``zoneinfo`` timezones near daylight-savings transitions (:issue:`49684`) +- Bug in :class:`DateOffset` when constructing with milliseconds in combination with another argument. An error was being raised because it was wrongly using relativedelta under the hood. - Timedelta From b3a5ef14a6ac1e87fe0e154bba3c5b7cf4576f7b Mon Sep 17 00:00:00 2001 From: Marko Pacak Date: Sat, 3 Dec 2022 14:22:50 +0100 Subject: [PATCH 04/17] DOC: update whatsnew --- doc/source/whatsnew/v2.0.0.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/source/whatsnew/v2.0.0.rst b/doc/source/whatsnew/v2.0.0.rst index 4c2791b173f40..eb37968e80938 100644 --- a/doc/source/whatsnew/v2.0.0.rst +++ b/doc/source/whatsnew/v2.0.0.rst @@ -638,7 +638,7 @@ Datetimelike - Bug in subtracting a ``datetime`` scalar from :class:`DatetimeIndex` failing to retain the original ``freq`` attribute (:issue:`48818`) - Bug in ``pandas.tseries.holiday.Holiday`` where a half-open date interval causes inconsistent return types from :meth:`USFederalHolidayCalendar.holidays` (:issue:`49075`) - Bug in rendering :class:`DatetimeIndex` and :class:`Series` and :class:`DataFrame` with timezone-aware dtypes with ``dateutil`` or ``zoneinfo`` timezones near daylight-savings transitions (:issue:`49684`) -- Bug in :class:`DateOffset` when constructing with milliseconds in combination with another argument. An error was being raised because it was wrongly using relativedelta under the hood. +- Bug in :class:`DateOffset` when constructing with milliseconds in combination with another argument (:issue:`49897`) - Timedelta From 705d2d654fa7b7706bc12de794165d132f61b7c2 Mon Sep 17 00:00:00 2001 From: Marko Pacak Date: Sun, 4 Dec 2022 16:28:44 +0100 Subject: [PATCH 05/17] CLN: reduce docstring --- pandas/tests/tseries/offsets/test_offsets.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/pandas/tests/tseries/offsets/test_offsets.py b/pandas/tests/tseries/offsets/test_offsets.py index 4a34d0bf6a516..cd9674e9d3836 100644 --- a/pandas/tests/tseries/offsets/test_offsets.py +++ b/pandas/tests/tseries/offsets/test_offsets.py @@ -740,11 +740,7 @@ def test_eq(self): assert DateOffset(milliseconds=3) != DateOffset(milliseconds=7) def test_milliseconds_combination(self): - """ - The combination of 'milliseconds' (plural) and another parameter - should not raise an error. - See https://github.com/pytest-dev/pytest/issues/10525 - """ + # 10525 DateOffset(days=1, milliseconds=1) From c1be7c6fc358b7d639b05e8a80129f625fc9c366 Mon Sep 17 00:00:00 2001 From: Marko Pacak Date: Sun, 4 Dec 2022 16:37:01 +0100 Subject: [PATCH 06/17] CLN: remove return statement --- pandas/_libs/tslibs/offsets.pyx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/pandas/_libs/tslibs/offsets.pyx b/pandas/_libs/tslibs/offsets.pyx index 05201660e8f16..5fb3857775d0f 100644 --- a/pandas/_libs/tslibs/offsets.pyx +++ b/pandas/_libs/tslibs/offsets.pyx @@ -315,9 +315,8 @@ cdef _determine_offset(kwds): use_relativedelta = False if len(kwds_no_nanos) > 0: if "milliseconds" in kwds_no_nanos: - return timedelta(**kwds_no_nanos), use_relativedelta - - if any(k in _kwds_use_relativedelta for k in kwds_no_nanos): + offset = timedelta(**kwds_no_nanos) + elif any(k in _kwds_use_relativedelta for k in kwds_no_nanos): if "millisecond" in kwds_no_nanos: raise NotImplementedError( "Using DateOffset to replace `millisecond` component in " From 64c9ee37a3ab235495317b1519dce673f0ddd5f7 Mon Sep 17 00:00:00 2001 From: Marko Pacak Date: Thu, 8 Dec 2022 11:41:45 +0100 Subject: [PATCH 07/17] TST: update assertions --- pandas/tests/tseries/offsets/test_offsets.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pandas/tests/tseries/offsets/test_offsets.py b/pandas/tests/tseries/offsets/test_offsets.py index cd9674e9d3836..2c0a047b29571 100644 --- a/pandas/tests/tseries/offsets/test_offsets.py +++ b/pandas/tests/tseries/offsets/test_offsets.py @@ -740,8 +740,10 @@ def test_eq(self): assert DateOffset(milliseconds=3) != DateOffset(milliseconds=7) def test_milliseconds_combination(self): - # 10525 - DateOffset(days=1, milliseconds=1) + # GH 10525 + offset = DateOffset(days=1, milliseconds=1) + assert offset.days == 1 + assert offset.milliseconds == 1 class TestOffsetNames: From 65015c80daca4812f8f097a60f843d6dd53d9487 Mon Sep 17 00:00:00 2001 From: Marko Pacak Date: Thu, 8 Dec 2022 22:02:49 +0100 Subject: [PATCH 08/17] CLN: rework _determine_offset and add tests --- pandas/_libs/tslibs/offsets.pyx | 80 ++++++++++---------- pandas/tests/tseries/offsets/test_offsets.py | 18 ++++- 2 files changed, 55 insertions(+), 43 deletions(-) diff --git a/pandas/_libs/tslibs/offsets.pyx b/pandas/_libs/tslibs/offsets.pyx index b169f9a457d37..77fbc61d0088d 100644 --- a/pandas/_libs/tslibs/offsets.pyx +++ b/pandas/_libs/tslibs/offsets.pyx @@ -297,46 +297,48 @@ _relativedelta_kwds = {"years", "months", "weeks", "days", "year", "month", "milliseconds", "microseconds"} -cdef _determine_offset(kwds): - # timedelta is used for sub-daily plural offsets and all singular - # offsets, relativedelta is used for plural offsets of daily length or - # more, nanosecond(s) are handled by apply_wraps - kwds_no_nanos = dict( - (k, v) for k, v in kwds.items() - if k not in ("nanosecond", "nanoseconds") - ) - # TODO: Are nanosecond and nanoseconds allowed somewhere? - - _kwds_use_relativedelta = ("years", "months", "weeks", "days", - "year", "month", "week", "day", "weekday", - "hour", "minute", "second", "microsecond", - "millisecond") - - use_relativedelta = False - if len(kwds_no_nanos) > 0: - if "milliseconds" in kwds_no_nanos: - offset = timedelta(**kwds_no_nanos) - elif any(k in _kwds_use_relativedelta for k in kwds_no_nanos): - if "millisecond" in kwds_no_nanos: - raise NotImplementedError( - "Using DateOffset to replace `millisecond` component in " - "datetime object is not supported. Use " - "`microsecond=timestamp.microsecond % 1000 + ms * 1000` " - "instead." - ) - offset = relativedelta(**kwds_no_nanos) - use_relativedelta = True - else: - # sub-daily offset - use timedelta (tz-aware) - offset = timedelta(**kwds_no_nanos) - elif any(nano in kwds for nano in ("nanosecond", "nanoseconds")): - offset = timedelta(days=0) - else: - # GH 45643/45890: (historically) defaults to 1 day for non-nano - # since datetime.timedelta doesn't handle nanoseconds - offset = timedelta(days=1) - return offset, use_relativedelta +def _determine_offset(kwds): + if not kwds: + # GH 45643/45890: (historically) defaults to 1 day + return timedelta(days=1), False + + _nanos = {"nanosecond", "nanoseconds"} + + # nanos are handled by apply_wraps + if all(k in _nanos for k in kwds): + return timedelta(days=0), False + + kwds_no_nanos = {k: v for k, v in kwds.items() if k not in _nanos} + + # Special case: "millisecond" (sing) + # TODO perhaps we can do the suggested conversion ourselves? + if "millisecond" in kwds_no_nanos: + raise NotImplementedError( + "Using DateOffset to replace `millisecond` component in " + "datetime object is not supported. Use " + "`microsecond=timestamp.microsecond % 1000 + ms * 1000` " + "instead." + ) + kwds_use_relativedelta = { + "year", "month", "day", "hour", "minute", + "second", "microsecond", "weekday", "years", "months", "weeks", "days", + "hours", "seconds", "microseconds" + } + kwds_use_timedelta = { + "days", "seconds", "microseconds", "milliseconds", "minutes", "hours", "weeks" + } + + if all(k in kwds_use_relativedelta for k in kwds_no_nanos): + return relativedelta(**kwds_no_nanos), True + + if all(k in kwds_use_timedelta for k in kwds_no_nanos): + # Sub-daily offset - use timedelta (tz-aware). + # This also handles "milliseconds" (plur): see GH 49897 + return timedelta(**kwds_no_nanos), False + + # TODO what to do in this case? + return timedelta(days=0), False # --------------------------------------------------------------------- # Mixins & Singletons diff --git a/pandas/tests/tseries/offsets/test_offsets.py b/pandas/tests/tseries/offsets/test_offsets.py index 2c0a047b29571..98e9f80c78c39 100644 --- a/pandas/tests/tseries/offsets/test_offsets.py +++ b/pandas/tests/tseries/offsets/test_offsets.py @@ -739,11 +739,21 @@ def test_eq(self): assert DateOffset(milliseconds=3) != DateOffset(milliseconds=7) - def test_milliseconds_combination(self): + @pytest.mark.parametrize( + "offset_kwargs, expected_arg", + [ + ({"days": 1, "milliseconds": 1}, "2022-01-02 00:0:00.001"), + ({"month": 1, "milliseconds": 1}, "2022-02-01 00:00:00.001"), + ], + ) + def test_milliseconds_combination(self, offset_kwargs, expected_arg): # GH 10525 - offset = DateOffset(days=1, milliseconds=1) - assert offset.days == 1 - assert offset.milliseconds == 1 + offset = DateOffset(**offset_kwargs) + ts = Timestamp("2022-01-01") + result = ts + offset + expected = Timestamp(expected_arg) + + assert result == expected class TestOffsetNames: From ef7f11884383b59fbb000a3f50597e971d292932 Mon Sep 17 00:00:00 2001 From: Marko Pacak Date: Thu, 8 Dec 2022 22:45:07 +0100 Subject: [PATCH 09/17] CLN: def to cdef --- pandas/_libs/tslibs/offsets.pyx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pandas/_libs/tslibs/offsets.pyx b/pandas/_libs/tslibs/offsets.pyx index 77fbc61d0088d..98b2e12e10a0c 100644 --- a/pandas/_libs/tslibs/offsets.pyx +++ b/pandas/_libs/tslibs/offsets.pyx @@ -297,7 +297,7 @@ _relativedelta_kwds = {"years", "months", "weeks", "days", "year", "month", "milliseconds", "microseconds"} -def _determine_offset(kwds): +cdef _determine_offset(kwds): if not kwds: # GH 45643/45890: (historically) defaults to 1 day return timedelta(days=1), False @@ -310,7 +310,6 @@ def _determine_offset(kwds): kwds_no_nanos = {k: v for k, v in kwds.items() if k not in _nanos} - # Special case: "millisecond" (sing) # TODO perhaps we can do the suggested conversion ourselves? if "millisecond" in kwds_no_nanos: raise NotImplementedError( From b2b7473377bb61e4f15d8f5ae06c66c21c50fdc2 Mon Sep 17 00:00:00 2001 From: Marko Pacak Date: Thu, 15 Dec 2022 12:37:01 +0100 Subject: [PATCH 10/17] BUG: GH49897 raise for invalid arguments + test --- pandas/_libs/tslibs/offsets.pyx | 6 +++--- pandas/tests/tseries/offsets/test_offsets.py | 15 ++++++++++++++- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/pandas/_libs/tslibs/offsets.pyx b/pandas/_libs/tslibs/offsets.pyx index 98b2e12e10a0c..b974bc3f77eb8 100644 --- a/pandas/_libs/tslibs/offsets.pyx +++ b/pandas/_libs/tslibs/offsets.pyx @@ -310,7 +310,6 @@ cdef _determine_offset(kwds): kwds_no_nanos = {k: v for k, v in kwds.items() if k not in _nanos} - # TODO perhaps we can do the suggested conversion ourselves? if "millisecond" in kwds_no_nanos: raise NotImplementedError( "Using DateOffset to replace `millisecond` component in " @@ -336,8 +335,9 @@ cdef _determine_offset(kwds): # This also handles "milliseconds" (plur): see GH 49897 return timedelta(**kwds_no_nanos), False - # TODO what to do in this case? - return timedelta(days=0), False + raise ValueError( + f"Invalid argument/s or bad combination of arguments: {list(kwds.keys())}" + ) # --------------------------------------------------------------------- # Mixins & Singletons diff --git a/pandas/tests/tseries/offsets/test_offsets.py b/pandas/tests/tseries/offsets/test_offsets.py index 98e9f80c78c39..31a00215b7267 100644 --- a/pandas/tests/tseries/offsets/test_offsets.py +++ b/pandas/tests/tseries/offsets/test_offsets.py @@ -743,7 +743,7 @@ def test_eq(self): "offset_kwargs, expected_arg", [ ({"days": 1, "milliseconds": 1}, "2022-01-02 00:0:00.001"), - ({"month": 1, "milliseconds": 1}, "2022-02-01 00:00:00.001"), + ({"hours": 1, "milliseconds": 1}, "2022-01-01 01:00:00.001"), ], ) def test_milliseconds_combination(self, offset_kwargs, expected_arg): @@ -755,6 +755,19 @@ def test_milliseconds_combination(self, offset_kwargs, expected_arg): assert result == expected + @pytest.mark.parametrize( + "offset_kwargs", + [ + ({"months": 1, "milliseconds": 1}), + ({"months": 1, "minutes": 1}), + ], + ) + def test_milliseconds_bad_combination(self, offset_kwargs): + # GH 10525 + msg_re = "^Invalid argument/s or bad combination of arguments:" + with pytest.raises(ValueError, match=msg_re): + DateOffset(**offset_kwargs) + class TestOffsetNames: def test_get_offset_name(self): From c2cf081204929f663b7d5c0af0e2c2fc3426c978 Mon Sep 17 00:00:00 2001 From: Marko Pacak Date: Mon, 19 Dec 2022 09:01:56 +0000 Subject: [PATCH 11/17] TST: GH 49897 remove useless test --- pandas/_libs/tslibs/offsets.pyx | 22 ++++++++++++-------- pandas/tests/tseries/offsets/test_offsets.py | 19 ++++------------- 2 files changed, 17 insertions(+), 24 deletions(-) diff --git a/pandas/_libs/tslibs/offsets.pyx b/pandas/_libs/tslibs/offsets.pyx index b974bc3f77eb8..4aa89cd4c9f25 100644 --- a/pandas/_libs/tslibs/offsets.pyx +++ b/pandas/_libs/tslibs/offsets.pyx @@ -302,13 +302,13 @@ cdef _determine_offset(kwds): # GH 45643/45890: (historically) defaults to 1 day return timedelta(days=1), False - _nanos = {"nanosecond", "nanoseconds"} + nanos = {"nanosecond", "nanoseconds"} # nanos are handled by apply_wraps - if all(k in _nanos for k in kwds): + if all(k in nanos for k in kwds): return timedelta(days=0), False - kwds_no_nanos = {k: v for k, v in kwds.items() if k not in _nanos} + kwds_no_nanos = {k: v for k, v in kwds.items() if k not in nanos} if "millisecond" in kwds_no_nanos: raise NotImplementedError( @@ -321,20 +321,24 @@ cdef _determine_offset(kwds): kwds_use_relativedelta = { "year", "month", "day", "hour", "minute", "second", "microsecond", "weekday", "years", "months", "weeks", "days", - "hours", "seconds", "microseconds" + "hours", "minutes", "seconds", "microseconds" } + + # "weeks" and "days" are left out despite being valid args for timedelta, + # because (historically) timedelta is used only for sub-daily. kwds_use_timedelta = { - "days", "seconds", "microseconds", "milliseconds", "minutes", "hours", "weeks" + "seconds", "microseconds", "milliseconds", "minutes", "hours", } - if all(k in kwds_use_relativedelta for k in kwds_no_nanos): - return relativedelta(**kwds_no_nanos), True - + # check timedelta before relativedelta: maintain compatibility with old tests if all(k in kwds_use_timedelta for k in kwds_no_nanos): - # Sub-daily offset - use timedelta (tz-aware). + # Sub-daily offset - use timedelta (tz-aware) # This also handles "milliseconds" (plur): see GH 49897 return timedelta(**kwds_no_nanos), False + if all(k in kwds_use_relativedelta for k in kwds_no_nanos): + return relativedelta(**kwds_no_nanos), True + raise ValueError( f"Invalid argument/s or bad combination of arguments: {list(kwds.keys())}" ) diff --git a/pandas/tests/tseries/offsets/test_offsets.py b/pandas/tests/tseries/offsets/test_offsets.py index 31a00215b7267..4bd20d0fe7528 100644 --- a/pandas/tests/tseries/offsets/test_offsets.py +++ b/pandas/tests/tseries/offsets/test_offsets.py @@ -742,12 +742,14 @@ def test_eq(self): @pytest.mark.parametrize( "offset_kwargs, expected_arg", [ - ({"days": 1, "milliseconds": 1}, "2022-01-02 00:0:00.001"), + ({"minutes": 1, "milliseconds": 1}, "2022-01-01 00:01:00.001"), ({"hours": 1, "milliseconds": 1}, "2022-01-01 01:00:00.001"), + ({"days": 1, "milliseconds": 1}, "2022-01-02 00:00:00.001"), + ({"weeks": 1, "milliseconds": 1}, "2022-01-08 00:00:00.001"), ], ) def test_milliseconds_combination(self, offset_kwargs, expected_arg): - # GH 10525 + # GH 49897 offset = DateOffset(**offset_kwargs) ts = Timestamp("2022-01-01") result = ts + offset @@ -755,19 +757,6 @@ def test_milliseconds_combination(self, offset_kwargs, expected_arg): assert result == expected - @pytest.mark.parametrize( - "offset_kwargs", - [ - ({"months": 1, "milliseconds": 1}), - ({"months": 1, "minutes": 1}), - ], - ) - def test_milliseconds_bad_combination(self, offset_kwargs): - # GH 10525 - msg_re = "^Invalid argument/s or bad combination of arguments:" - with pytest.raises(ValueError, match=msg_re): - DateOffset(**offset_kwargs) - class TestOffsetNames: def test_get_offset_name(self): From 3981b04738af3c50353f763d03c8c5310a257575 Mon Sep 17 00:00:00 2001 From: Marko Pacak Date: Mon, 19 Dec 2022 09:06:32 +0000 Subject: [PATCH 12/17] TST: BUG49897 convert millis to micros so relativedelta can use them --- pandas/_libs/tslibs/offsets.pyx | 5 +++++ pandas/tests/tseries/offsets/test_offsets.py | 3 +++ 2 files changed, 8 insertions(+) diff --git a/pandas/_libs/tslibs/offsets.pyx b/pandas/_libs/tslibs/offsets.pyx index 4aa89cd4c9f25..9ce5a73ec9b5f 100644 --- a/pandas/_libs/tslibs/offsets.pyx +++ b/pandas/_libs/tslibs/offsets.pyx @@ -336,6 +336,11 @@ cdef _determine_offset(kwds): # This also handles "milliseconds" (plur): see GH 49897 return timedelta(**kwds_no_nanos), False + # convert milliseconds to microseconds, so relativedelta can parse it + if "milliseconds" in kwds_no_nanos: + micro = kwds_no_nanos.pop("milliseconds") * 1000 + kwds_no_nanos["microseconds"] = kwds_no_nanos.get("microseconds", 0) + micro + if all(k in kwds_use_relativedelta for k in kwds_no_nanos): return relativedelta(**kwds_no_nanos), True diff --git a/pandas/tests/tseries/offsets/test_offsets.py b/pandas/tests/tseries/offsets/test_offsets.py index 4bd20d0fe7528..5ba386abd7029 100644 --- a/pandas/tests/tseries/offsets/test_offsets.py +++ b/pandas/tests/tseries/offsets/test_offsets.py @@ -742,10 +742,13 @@ def test_eq(self): @pytest.mark.parametrize( "offset_kwargs, expected_arg", [ + ({"seconds": 1, "milliseconds": 1}, "2022-01-01 00:00:01.001"), ({"minutes": 1, "milliseconds": 1}, "2022-01-01 00:01:00.001"), ({"hours": 1, "milliseconds": 1}, "2022-01-01 01:00:00.001"), ({"days": 1, "milliseconds": 1}, "2022-01-02 00:00:00.001"), ({"weeks": 1, "milliseconds": 1}, "2022-01-08 00:00:00.001"), + ({"months": 1, "milliseconds": 1}, "2022-02-01 00:00:00.001"), + ({"years": 1, "milliseconds": 1}, "2023-01-01 00:00:00.001"), ], ) def test_milliseconds_combination(self, offset_kwargs, expected_arg): From 27109a7f302f3494d243c93ec4cf770515e1ddd1 Mon Sep 17 00:00:00 2001 From: Marko Pacak Date: Thu, 22 Dec 2022 08:26:08 +0000 Subject: [PATCH 13/17] TST: GH49897 test millis + micros --- pandas/tests/tseries/offsets/test_offsets.py | 1 + 1 file changed, 1 insertion(+) diff --git a/pandas/tests/tseries/offsets/test_offsets.py b/pandas/tests/tseries/offsets/test_offsets.py index 5ba386abd7029..a35120e86125e 100644 --- a/pandas/tests/tseries/offsets/test_offsets.py +++ b/pandas/tests/tseries/offsets/test_offsets.py @@ -742,6 +742,7 @@ def test_eq(self): @pytest.mark.parametrize( "offset_kwargs, expected_arg", [ + ({"microseconds": 1, "milliseconds": 1}, "2022-01-01 00:00:00.001001"), ({"seconds": 1, "milliseconds": 1}, "2022-01-01 00:00:01.001"), ({"minutes": 1, "milliseconds": 1}, "2022-01-01 00:01:00.001"), ({"hours": 1, "milliseconds": 1}, "2022-01-01 01:00:00.001"), From 2ad8c60313db4d260d053e00c14b66c6f95b99f8 Mon Sep 17 00:00:00 2001 From: Marko Pacak Date: Thu, 22 Dec 2022 09:10:35 +0000 Subject: [PATCH 14/17] CLN: GH49897 remove comment --- pandas/_libs/tslibs/offsets.pyx | 1 - 1 file changed, 1 deletion(-) diff --git a/pandas/_libs/tslibs/offsets.pyx b/pandas/_libs/tslibs/offsets.pyx index 130c1375a6b59..521635c722c15 100644 --- a/pandas/_libs/tslibs/offsets.pyx +++ b/pandas/_libs/tslibs/offsets.pyx @@ -330,7 +330,6 @@ cdef _determine_offset(kwds): "seconds", "microseconds", "milliseconds", "minutes", "hours", } - # check timedelta before relativedelta: maintain compatibility with old tests if all(k in kwds_use_timedelta for k in kwds_no_nanos): # Sub-daily offset - use timedelta (tz-aware) # This also handles "milliseconds" (plur): see GH 49897 From 8eb83d0ad3c895bd488fbf94caaa7bea2b3430de Mon Sep 17 00:00:00 2001 From: Marko Pacak Date: Thu, 22 Dec 2022 10:10:05 +0000 Subject: [PATCH 15/17] DOC: GH49897 update rst --- doc/source/whatsnew/v2.0.0.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/source/whatsnew/v2.0.0.rst b/doc/source/whatsnew/v2.0.0.rst index ecfe0f739e530..99ed779487c34 100644 --- a/doc/source/whatsnew/v2.0.0.rst +++ b/doc/source/whatsnew/v2.0.0.rst @@ -799,7 +799,7 @@ Datetimelike - Bug in :class:`Timestamp` was showing ``UserWarning``, which was not actionable by users, when parsing non-ISO8601 delimited date strings (:issue:`50232`) - Bug in :func:`to_datetime` was showing misleading ``ValueError`` when parsing dates with format containing ISO week directive and ISO weekday directive (:issue:`50308`) - Bug in :func:`to_datetime` was not raising ``ValueError`` when invalid format was passed and ``errors`` was ``'ignore'`` or ``'coerce'`` (:issue:`50266`) -- Bug in :class:`DateOffset` when constructing with milliseconds and another super-daily argument. A ``TypeError`` was raised due to misuse of underlying :class:`datetime.timedelta` and :class:`dateutil.relativedelta.relativedelta` (:issue:`49897`) +- Bug in :class:`DateOffset` was throwing ``TypeError`` when constructing with milliseconds and another super-daily argument (:issue:`49897`) - Timedelta From 309e29683850139cd0e2d65979db1c5b5aed0477 Mon Sep 17 00:00:00 2001 From: Marko Pacak Date: Thu, 22 Dec 2022 17:12:52 +0000 Subject: [PATCH 16/17] CLN: re-order if-conditions --- pandas/_libs/tslibs/offsets.pyx | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/pandas/_libs/tslibs/offsets.pyx b/pandas/_libs/tslibs/offsets.pyx index 521635c722c15..470d1e89e5b88 100644 --- a/pandas/_libs/tslibs/offsets.pyx +++ b/pandas/_libs/tslibs/offsets.pyx @@ -302,6 +302,14 @@ cdef _determine_offset(kwds): # GH 45643/45890: (historically) defaults to 1 day return timedelta(days=1), False + if "millisecond" in kwds: + raise NotImplementedError( + "Using DateOffset to replace `millisecond` component in " + "datetime object is not supported. Use " + "`microsecond=timestamp.microsecond % 1000 + ms * 1000` " + "instead." + ) + nanos = {"nanosecond", "nanoseconds"} # nanos are handled by apply_wraps @@ -310,14 +318,6 @@ cdef _determine_offset(kwds): kwds_no_nanos = {k: v for k, v in kwds.items() if k not in nanos} - if "millisecond" in kwds_no_nanos: - raise NotImplementedError( - "Using DateOffset to replace `millisecond` component in " - "datetime object is not supported. Use " - "`microsecond=timestamp.microsecond % 1000 + ms * 1000` " - "instead." - ) - kwds_use_relativedelta = { "year", "month", "day", "hour", "minute", "second", "microsecond", "weekday", "years", "months", "weeks", "days", From 0d85a740ed6248eae89b7ba6e0e1de784944589e Mon Sep 17 00:00:00 2001 From: Marko Pacak Date: Thu, 22 Dec 2022 20:00:23 +0000 Subject: [PATCH 17/17] TST: GH49897 test raise --- pandas/tests/tseries/offsets/test_offsets.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pandas/tests/tseries/offsets/test_offsets.py b/pandas/tests/tseries/offsets/test_offsets.py index a35120e86125e..135227d66d541 100644 --- a/pandas/tests/tseries/offsets/test_offsets.py +++ b/pandas/tests/tseries/offsets/test_offsets.py @@ -761,6 +761,11 @@ def test_milliseconds_combination(self, offset_kwargs, expected_arg): assert result == expected + def test_offset_invalid_arguments(self): + msg = "^Invalid argument/s or bad combination of arguments" + with pytest.raises(ValueError, match=msg): + DateOffset(picoseconds=1) + class TestOffsetNames: def test_get_offset_name(self):