BUG: args offset in Timestamp.__new__ causes bugs

asv_bench/benchmarks/tslibs/timestamp.py

@@ -48,6 +48,9 @@ def time_from_datetime_aware(self):
     def time_from_pd_timestamp(self):
         Timestamp(self.ts)
 
+    def time_from_positional(self):
+        Timestamp(2020, 1, 1, 0, 0, 0)
+
 
 class TimestampProperties:
     params = [_tzs]

doc/source/whatsnew/v2.1.0.rst

@@ -165,6 +165,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

pandas/_libs/tslibs/timestamps.pyx

@@ -1520,31 +1520,14 @@ class Timestamp(_Timestamp):
         """
         return cls(datetime.combine(date, time))
 
-    def __new__(
-        cls,
-        object ts_input=_no_input,
-        year=None,
-        month=None,
-        day=None,
-        hour=None,
-        minute=None,
-        second=None,
-        microsecond=None,
-        tzinfo_type tzinfo=None,
-        *,
-        nanosecond=None,
-        tz=None,
-        unit=None,
-        fold=None,
-    ):
+    def __new__(cls, *args, **kwargs):
         # The parameter list folds together legacy parameter names (the first
         # four) and positional and keyword parameter names from pydatetime.
         #
         # There are three calling forms:
         #
         # - In the legacy form, the first parameter, ts_input, is required
-        #   and may be datetime-like, str, int, or float. The second
-        #   parameter, offset, is optional and may be str or DateOffset.
+        #   and may be datetime-like, str, int, or float.
         #
         # - ints in the first, second, and third arguments indicate
         #   pydatetime positional arguments. Only the first 8 arguments
@@ -1553,17 +1536,81 @@ class Timestamp(_Timestamp):
         #   check that the second argument is an int.
         #
         # - Nones for the first four (legacy) arguments indicate pydatetime
-        #   keyword arguments. year, month, and day are required. As a
-        #   shortcut, we just check that the first argument was not passed.
-        #
-        # Mixing pydatetime positional and keyword arguments is forbidden!
+        #   keyword arguments. year, month, and day are required. We just
+        #   check that no positional arguments were passed.
 
         cdef:
+            object ts_input=_no_input
             _TSObject ts
-            tzinfo_type tzobj
+            tzinfo_type tzinfo, tzobj
 
-        _date_attributes = [year, month, day, hour, minute, second,
-                            microsecond, nanosecond]
+        args_len = len(args)
+
+        # GH 30543 if pd.Timestamp already passed, return it
+        # check that only ts_input is passed
+        # checking verbosely, because cython doesn't optimize
+        # list comprehensions (as of cython 0.29.x)
+        if args_len == 1 and len(kwargs) == 0 and isinstance(args[0], _Timestamp):
+            return args[0]
+
+        # Check that kwargs weren't passed as args
+        # if args_len > 9:
+        #     raise ValueError(invalid_args_msg)
+
+        # Unpack the arguments
+        # Building from ts_input
+        if args_len == 1:
+            if kwargs:
+                if ("year" in kwargs or "month" in kwargs or "day" in kwargs or
+                        "hour" in kwargs or "minute" in kwargs or "second" in kwargs or
+                        "microsecond" in kwargs):
+                    raise ValueError("Cannot pass a date attribute keyword argument")
+                if isinstance(args[0], str) and "nanosecond" in kwargs:
+                    raise ValueError(
+                        "Cannot pass a date attribute keyword "
+                        "argument when passing a date string; 'tz' is keyword-only"
+                    )
+
+            ts_input = args[0]
+            tzinfo = kwargs.get("tzinfo")
+        # Building from positional arguments
+        elif 9 > args_len > 2 and isinstance(args[1], int):
+            args = args + (None,) * (8 - args_len)
+            year, month, day, hour, minute, second, microsecond, tzinfo = args
+
+            if kwargs:
+                # Positional or keyword arguments
+                hour = kwargs.get("hour", hour)
+                minute = kwargs.get("minute", minute)
+                second = kwargs.get("second", second)
+                microsecond = kwargs.get("microsecond", microsecond)
+                tzinfo = kwargs.get("tzinfo", tzinfo)
+        # Keywords only
+        elif args_len == 0:
+            ts_input = kwargs.get("ts_input", _no_input)
+            year = kwargs.get("year")
+            month = kwargs.get("month")
+            day = kwargs.get("day")
+            hour = kwargs.get("hour")
+            minute = kwargs.get("minute")
+            second = kwargs.get("second")
+            microsecond = kwargs.get("microsecond")
+            tzinfo = kwargs.get("tzinfo")
+        # kludge for reading legacy pickle with read_pickle in test_pickle
+        elif (args_len == 3 and isinstance(args[0], int) and
+              (not isinstance(args[1], int) or not isinstance(args[2], int))):
+            ts_input = args[0]
+            tzinfo = args[-1]
+        else:
+            raise ValueError(
+                f"Invalid Timestamp arguments. args: {args}, kwargs: {kwargs}"
+            )
+
+        # Unpack keyword-only arguments
+        nanosecond = kwargs.get("nanosecond", 0)
+        tz = kwargs.get("tz")
+        unit = kwargs.get("unit")
+        fold = kwargs.get("fold")
 
         if tzinfo is not None:
             # GH#17690 tzinfo must be a datetime.tzinfo object, ensured
@@ -1600,27 +1647,7 @@ class Timestamp(_Timestamp):
             if hasattr(ts_input, "fold"):
                 ts_input = ts_input.replace(fold=fold)
 
-        # GH 30543 if pd.Timestamp already passed, return it
-        # check that only ts_input is passed
-        # checking verbosely, because cython doesn't optimize
-        # list comprehensions (as of cython 0.29.x)
-        if (isinstance(ts_input, _Timestamp) and
-                tz is None and unit is None and year is None and
-                month is None and day is None and hour is None and
-                minute is None and second is None and
-                microsecond is None and nanosecond is None and
-                tzinfo is None):
-            return ts_input
-        elif isinstance(ts_input, str):
-            # User passed a date string to parse.
-            # Check that the user didn't also pass a date attribute kwarg.
-            if any(arg is not None for arg in _date_attributes):
-                raise ValueError(
-                    "Cannot pass a date attribute keyword "
-                    "argument when passing a date string; 'tz' is keyword-only"
-                )
-
-        elif ts_input is _no_input:
+        if ts_input is _no_input:
             # GH 31200
             # When year, month or day is not given, we call the datetime
             # constructor to make sure we get the same error message
@@ -1641,14 +1668,6 @@ class Timestamp(_Timestamp):
 
             ts_input = datetime(**datetime_kwargs)
 
-        elif is_integer_object(year):
-            # User passed positional arguments:
-            # Timestamp(year, month, day[, hour[, minute[, second[,
-            # microsecond[, tzinfo]]]]])
-            ts_input = datetime(ts_input, year, month, day or 0,
-                                hour or 0, minute or 0, second or 0, fold=fold or 0)
-            unit = None
-
         if getattr(ts_input, "tzinfo", None) is not None and tz is not None:
             raise ValueError("Cannot pass a datetime or Timestamp with tzinfo with "
                              "the tz parameter. Use tz_convert instead.")
@@ -1659,9 +1678,7 @@ class Timestamp(_Timestamp):
             #  wall-time (consistent with DatetimeIndex)
             return cls(ts_input).tz_localize(tzobj)
 
-        if nanosecond is None:
-            nanosecond = 0
-        elif not (999 >= nanosecond >= 0):
+        if not (999 >= nanosecond >= 0):
             raise ValueError("nanosecond must be in 0..999")
 
         ts = convert_to_tsobject(ts_input, tzobj, unit, 0, 0, nanosecond)

pandas/tests/scalar/timestamp/test_constructors.py

@@ -13,10 +13,7 @@
 import pytz
 
 from pandas._libs.tslibs.dtypes import NpyDatetimeUnit
-from pandas.compat import (
-    PY39,
-    PY310,
-)
+from pandas.compat import PY39
 from pandas.errors import OutOfBoundsDatetime
 
 from pandas import (
@@ -293,18 +290,19 @@ def test_constructor_invalid(self):
 
     def test_constructor_invalid_tz(self):
         # GH#17690
-        msg = (
-            "Argument 'tzinfo' has incorrect type "
-            r"\(expected datetime.tzinfo, got str\)"
-        )
+        msg = "Cannot convert str to datetime.tzinfo"
         with pytest.raises(TypeError, match=msg):
             Timestamp("2017-10-22", tzinfo="US/Eastern")
 
         msg = "at most one of"
         with pytest.raises(ValueError, match=msg):
             Timestamp("2017-10-22", tzinfo=pytz.utc, tz="UTC")
 
-        msg = "Cannot pass a date attribute keyword argument when passing a date string"
+        msg = (
+            "Invalid Timestamp arguments. "
+            "args: \\('2012-01-01', 'US/Pacific'\\), "
+            "kwargs: {}"
+        )
         with pytest.raises(ValueError, match=msg):
             # GH#5168
             # case where user tries to pass tz as an arg, not kwarg, gets
@@ -340,12 +338,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)
@@ -357,12 +350,8 @@ def test_constructor_positional_keyword_mixed_with_tzinfo(self, kwd, request):
 
     def test_constructor_positional(self):
         # see gh-10758
-        msg = (
-            "'NoneType' object cannot be interpreted as an integer"
-            if PY310
-            else "an integer is required"
-        )
-        with pytest.raises(TypeError, match=msg):
+        msg = "Invalid Timestamp arguments. args: \\(2000, 1\\), kwargs: {}"
+        with pytest.raises(ValueError, match=msg):
             Timestamp(2000, 1)
 
         msg = "month must be in 1..12"
@@ -891,3 +880,21 @@ 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
+
+
+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