From 9371f226fd6c0695c5f7c5ced832f46986379002 Mon Sep 17 00:00:00 2001 From: Alex Kirko Date: Sun, 26 Mar 2023 13:37:27 +0200 Subject: [PATCH 1/9] BUG: reformulate the predicate --- pandas/_libs/tslibs/timestamps.pyx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pandas/_libs/tslibs/timestamps.pyx b/pandas/_libs/tslibs/timestamps.pyx index 10a331f302cc4..be88a0254acdb 100644 --- a/pandas/_libs/tslibs/timestamps.pyx +++ b/pandas/_libs/tslibs/timestamps.pyx @@ -1581,9 +1581,9 @@ class Timestamp(_Timestamp): "Valid values for the fold argument are None, 0, or 1." ) - if (ts_input is not _no_input and not ( + if (ts_input is not _no_input and PyDateTime_Check(ts_input) and - getattr(ts_input, "tzinfo", None) is None)): + getattr(ts_input, "tzinfo", None) is not None): raise ValueError( "Cannot pass fold with possibly unambiguous input: int, " "float, numpy.datetime64, str, or timezone-aware " From b7b37ad19206a4c9e6858daecd618f2377921ce3 Mon Sep 17 00:00:00 2001 From: Alex Kirko Date: Sun, 26 Mar 2023 14:10:53 +0200 Subject: [PATCH 2/9] TST: add the test --- pandas/tests/scalar/timestamp/test_constructors.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/pandas/tests/scalar/timestamp/test_constructors.py b/pandas/tests/scalar/timestamp/test_constructors.py index 7e4002dc3a0cf..03f7babfcbe70 100644 --- a/pandas/tests/scalar/timestamp/test_constructors.py +++ b/pandas/tests/scalar/timestamp/test_constructors.py @@ -899,3 +899,13 @@ def test_timestamp_constructor_adjust_value_for_fold(tz, ts_input, fold, value_o result = ts._value expected = value_out assert result == expected + + +@pytest.mark.parametrize("tz", ["dateutil/Europe/London"]) +def test_timestamp_constructor_positional_with_fold(tz): + # Check that we build an object successfully + # if we pass positional arguments and fold + ts = Timestamp(2019, 10, 27, 1, 30, tz=tz, fold=0) + result = ts._value + expected = 1572136200000000 + assert result == expected From e97541d95607913853aa0fd8e12bdbc88238b0fe Mon Sep 17 00:00:00 2001 From: Alex Kirko Date: Sun, 26 Mar 2023 15:25:09 +0200 Subject: [PATCH 3/9] REFACT: mv from-positional inside the if --- pandas/_libs/tslibs/timestamps.pyx | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/pandas/_libs/tslibs/timestamps.pyx b/pandas/_libs/tslibs/timestamps.pyx index be88a0254acdb..4d8910070bb00 100644 --- a/pandas/_libs/tslibs/timestamps.pyx +++ b/pandas/_libs/tslibs/timestamps.pyx @@ -1581,9 +1581,14 @@ class Timestamp(_Timestamp): "Valid values for the fold argument are None, 0, or 1." ) - if (ts_input is not _no_input and + # GH#52117 note that currently we end up with the year in the ts_input + # variable when using positional arguments. We should fix this. + from_positional = (ts_input is not None and + year is not None and month is not None) + + if (ts_input is not _no_input and not from_positional and not ( PyDateTime_Check(ts_input) and - getattr(ts_input, "tzinfo", None) is not None): + getattr(ts_input, "tzinfo", None) is None)): raise ValueError( "Cannot pass fold with possibly unambiguous input: int, " "float, numpy.datetime64, str, or timezone-aware " From b3282a59687d6119c566d4701aba981b105480ce Mon Sep 17 00:00:00 2001 From: Alex Kirko Date: Sun, 26 Mar 2023 16:10:02 +0200 Subject: [PATCH 4/9] DOC: add whatsnew --- 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 71fda39a05e55..4c56cebe60383 100644 --- a/doc/source/whatsnew/v2.1.0.rst +++ b/doc/source/whatsnew/v2.1.0.rst @@ -153,6 +153,7 @@ Datetimelike - Bug in :meth:`Timestamp.round` with values close to the implementation bounds returning incorrect results instead of raising ``OutOfBoundsDatetime`` (:issue:`51494`) - :meth:`arrays.DatetimeArray.map` can now take a ``na_action`` argument. :meth:`DatetimeIndex.map` with ``na_action="ignore"`` now works as expected. (:issue:`51644`) - Bug in :meth:`arrays.DatetimeArray.map` and :meth:`DatetimeIndex.map`, where the supplied callable operated array-wise instead of element-wise (:issue:`51977`) +- Bug in :class:`Timestamp` raising an error when passing fold when constructing from positional arguments. - Timedelta From 6ee4770eab10772131a407839b31997301c67a1e Mon Sep 17 00:00:00 2001 From: Alex Kirko Date: Mon, 27 Mar 2023 09:01:14 +0200 Subject: [PATCH 5/9] attempt to fix the core issue instead --- pandas/_libs/tslibs/timestamps.pyx | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/pandas/_libs/tslibs/timestamps.pyx b/pandas/_libs/tslibs/timestamps.pyx index 4d8910070bb00..91c0e56aa4232 100644 --- a/pandas/_libs/tslibs/timestamps.pyx +++ b/pandas/_libs/tslibs/timestamps.pyx @@ -1562,6 +1562,14 @@ class Timestamp(_Timestamp): _TSObject ts tzinfo_type tzobj + # GH#52117 If we passed positional args, then _ts_input + # is now set to year and the other positional args + # are shifted to the left. Let's shift back + if (isinstance(ts_input, int) and + isinstance(year, int) and isinstance(month, int)): + ts_input, year, month, day, hour, minute, second, microsecond = ( + _no_input, ts_input, year, month, day, hour, minute, second) + _date_attributes = [year, month, day, hour, minute, second, microsecond, nanosecond] @@ -1581,12 +1589,7 @@ class Timestamp(_Timestamp): "Valid values for the fold argument are None, 0, or 1." ) - # GH#52117 note that currently we end up with the year in the ts_input - # variable when using positional arguments. We should fix this. - from_positional = (ts_input is not None and - year is not None and month is not None) - - if (ts_input is not _no_input and not from_positional and not ( + if (ts_input is not _no_input and not ( PyDateTime_Check(ts_input) and getattr(ts_input, "tzinfo", None) is None)): raise ValueError( From e0699ba46c1eb7af2c2287acf37cb0fe03d12b61 Mon Sep 17 00:00:00 2001 From: Alex Kirko Date: Tue, 28 Mar 2023 19:32:02 +0200 Subject: [PATCH 6/9] BUG: mv the bugfix to decorator to allow for mixed arguments --- pandas/_libs/tslibs/timestamps.pyx | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/pandas/_libs/tslibs/timestamps.pyx b/pandas/_libs/tslibs/timestamps.pyx index 91c0e56aa4232..f3620cf006f51 100644 --- a/pandas/_libs/tslibs/timestamps.pyx +++ b/pandas/_libs/tslibs/timestamps.pyx @@ -1269,7 +1269,23 @@ cdef class _Timestamp(ABCTimestamp): # Python front end to C extension type _Timestamp # This serves as the box for datetime64 +def _fix_positional_arguments(cls): + original_new = cls.__new__ + def updated_new(cls, *args, **kwargs): + # GH#52117 If we passed positional args, then _ts_input + # is now set to year and the other positional args + # are shifted to the left. Let's shift back + if len(args) > 2 and all(isinstance(arg, int) for arg in args[:3]): + args = (_no_input,) + args + instance = original_new(cls, *args, **kwargs) + return instance + + cls.__new__ = updated_new + return cls + + +@_fix_positional_arguments class Timestamp(_Timestamp): """ Pandas replacement for python datetime.datetime object. @@ -1562,14 +1578,6 @@ class Timestamp(_Timestamp): _TSObject ts tzinfo_type tzobj - # GH#52117 If we passed positional args, then _ts_input - # is now set to year and the other positional args - # are shifted to the left. Let's shift back - if (isinstance(ts_input, int) and - isinstance(year, int) and isinstance(month, int)): - ts_input, year, month, day, hour, minute, second, microsecond = ( - _no_input, ts_input, year, month, day, hour, minute, second) - _date_attributes = [year, month, day, hour, minute, second, microsecond, nanosecond] From ebd419e5fad5d177881434c5ac25a5f5a8553a79 Mon Sep 17 00:00:00 2001 From: Alex Kirko Date: Tue, 28 Mar 2023 19:47:22 +0200 Subject: [PATCH 7/9] TST: update the tests now that behavior is consistent --- pandas/tests/scalar/timestamp/test_constructors.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/pandas/tests/scalar/timestamp/test_constructors.py b/pandas/tests/scalar/timestamp/test_constructors.py index 03f7babfcbe70..8f28f4d5a8fb1 100644 --- a/pandas/tests/scalar/timestamp/test_constructors.py +++ b/pandas/tests/scalar/timestamp/test_constructors.py @@ -340,12 +340,7 @@ def test_constructor_positional_with_tzinfo(self): @pytest.mark.parametrize("kwd", ["nanosecond", "microsecond", "second", "minute"]) def test_constructor_positional_keyword_mixed_with_tzinfo(self, kwd, request): - # TODO: if we passed microsecond with a keyword we would mess up - # xref GH#45307 - if kwd != "nanosecond": - # nanosecond is keyword-only as of 2.0, others are not - mark = pytest.mark.xfail(reason="GH#45307") - request.node.add_marker(mark) + # GH#52221 makes a mix of positional and keyword arguments behave consistently kwargs = {kwd: 4} ts = Timestamp(2020, 12, 31, tzinfo=timezone.utc, **kwargs) From 870dc4058aa5781f4b841ca03366563bfa0efa3d Mon Sep 17 00:00:00 2001 From: Alex Kirko Date: Tue, 28 Mar 2023 20:02:21 +0200 Subject: [PATCH 8/9] TST: add the argument mix test --- pandas/tests/scalar/timestamp/test_constructors.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pandas/tests/scalar/timestamp/test_constructors.py b/pandas/tests/scalar/timestamp/test_constructors.py index 8f28f4d5a8fb1..e80594fdc87d8 100644 --- a/pandas/tests/scalar/timestamp/test_constructors.py +++ b/pandas/tests/scalar/timestamp/test_constructors.py @@ -904,3 +904,11 @@ def test_timestamp_constructor_positional_with_fold(tz): result = ts._value expected = 1572136200000000 assert result == expected + + +def test_timestamp_constructor_arg_shift(): + # Check that passing a positional argument as keyword + # does not change the value + result = Timestamp(2019, 10, 27, minute=30) + expected = Timestamp(2019, 10, 27, 0, 30) + assert result == expected From 635e0ccef08b9ec1b0fe2448057261c09594f58a Mon Sep 17 00:00:00 2001 From: Alex Kirko Date: Tue, 28 Mar 2023 20:46:48 +0200 Subject: [PATCH 9/9] CLN: clean up the function a bit --- pandas/_libs/tslibs/timestamps.pyx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pandas/_libs/tslibs/timestamps.pyx b/pandas/_libs/tslibs/timestamps.pyx index f3620cf006f51..6307323d42f2f 100644 --- a/pandas/_libs/tslibs/timestamps.pyx +++ b/pandas/_libs/tslibs/timestamps.pyx @@ -1277,9 +1277,9 @@ def _fix_positional_arguments(cls): # is now set to year and the other positional args # are shifted to the left. Let's shift back if len(args) > 2 and all(isinstance(arg, int) for arg in args[:3]): - args = (_no_input,) + args - instance = original_new(cls, *args, **kwargs) - return instance + return original_new(cls, _no_input, *args, **kwargs) + else: + return original_new(cls, *args, **kwargs) cls.__new__ = updated_new return cls