Timestamp -> datetime and Timedelta -> timedelta #841

twoertwein · 2023-12-24T04:49:19Z

Closes BUG: Type stubs only support pd.Timedelta, but could (should?) also support datetime.timedelta #838 (Replace xxxx with the Github issue number)
Tests added: Please use assert_type() to assert the type of any return value

Could probably benefit by adding tests where the arguments became wider.

Dr-Irv

We had a discussion in #341 (comment) where we preferred redundancy for documentation purposes. So I think it would be better to have timedelta | Timedelta rather than timedelta and datetime | Timestamp instead of datetime.

The other thing I'm concerned about is that with this change we have widened all the accepted types for many methods, and we don't have tests to see if the wider types are accepted. For example, maybe there is some method that accepts pd.Timedelta, but not datetime.timedelta. But that would be a lot of work to duplicate all the tests we have that use pd.Timedelta and pd.Timestamp as arguments and see if they work with datetime.timedelta and datetime.datetime as well.

Open to discussion on both of these points.

Dr-Irv

Whenever I said "put back Timedelta", etc., I mean to include make it where it reads Timedelta | dt.timedelta (in case that wasn't clear)

Dr-Irv · 2023-12-26T15:14:22Z

pandas-stubs/_libs/tslibs/timedeltas.pyi

@@ -210,7 +210,7 @@ class Timedelta(timedelta):
    @overload
    def __rsub__(self, other: timedelta | Timedelta | np.timedelta64) -> Timedelta: ...
    @overload
-    def __rsub__(self, other: Timestamp | np.datetime64) -> Timestamp: ...
+    def __rsub__(self, other: dt.datetime | np.datetime64) -> Timestamp: ...  # type: ignore[misc]


put back Timestamp

Dr-Irv · 2023-12-26T15:14:40Z

pandas-stubs/_libs/tslibs/timestamps.pyi

+    @classmethod
+    def fromtimestamp(cls, t: float, tz: _tzinfo | str | None = ...) -> Self: ...


Need to rebase against main

Dr-Irv · 2023-12-26T15:15:14Z

pandas-stubs/core/frame.pyi

@@ -1941,7 +1940,7 @@ class DataFrame(NDFrame, OpsMixin):
        level: Level | None = ...,
        origin: Timestamp
        | Literal["epoch", "start", "start_day", "end", "end_day"] = ...,
-        offset: Timedelta | _str | None = ...,
+        offset: dt.timedelta | _str | None = ...,


put back Timedelta

Dr-Irv · 2023-12-26T15:15:43Z

pandas-stubs/core/indexes/accessors.pyi

@@ -166,21 +169,21 @@ class _DatetimeRoundingMethods(Generic[_DTRoundingMethodReturnType]):
        freq: str | BaseOffset | None,
        ambiguous: Literal["raise", "infer", "NaT"] | np_ndarray_bool = ...,
        nonexistent: Literal["shift_forward", "shift_backward", "NaT", "raise"]
-        | Timedelta = ...,
+        | timedelta = ...,


put back Timedelta

Dr-Irv · 2023-12-26T15:15:58Z

pandas-stubs/core/indexes/accessors.pyi

    ) -> _DTRoundingMethodReturnType: ...
    def floor(
        self,
        freq: str | BaseOffset | None,
        ambiguous: Literal["raise", "infer", "NaT"] | np_ndarray_bool = ...,
        nonexistent: Literal["shift_forward", "shift_backward", "NaT", "raise"]
-        | Timedelta = ...,
+        | timedelta = ...,


put back Timedelta

Dr-Irv · 2023-12-26T15:17:36Z

pandas-stubs/core/series.pyi

@@ -1485,7 +1485,7 @@ class Series(IndexOpsMixin[S1], NDFrame):
    ) -> Series[_bool]: ...
    @overload
    def __mul__(
-        self, other: Timedelta | TimedeltaSeries | np.timedelta64
+        self, other: timedelta | TimedeltaSeries | np.timedelta64


put back Timedelta

Dr-Irv · 2023-12-26T15:17:47Z

pandas-stubs/core/series.pyi

@@ -2043,18 +2043,18 @@ class TimedeltaSeries(Series[Timedelta]):
    def __add__(self, other: Period) -> PeriodSeries: ...
    @overload
    def __add__(
-        self, other: Timestamp | TimestampSeries | DatetimeIndex
+        self, other: datetime | TimestampSeries | DatetimeIndex


put back Timestamp

Dr-Irv · 2023-12-26T15:17:52Z

pandas-stubs/core/series.pyi

    ) -> TimestampSeries: ...
    @overload
    def __add__(  # pyright: ignore[reportIncompatibleMethodOverride]
-        self, other: Timedelta | np.timedelta64
+        self, other: timedelta | np.timedelta64


put back Timedelta

Dr-Irv · 2023-12-26T15:18:03Z

pandas-stubs/core/series.pyi

    ) -> TimedeltaSeries: ...
-    def __radd__(self, other: Timestamp | TimestampSeries) -> TimestampSeries: ...  # type: ignore[override] # pyright: ignore[reportIncompatibleMethodOverride]
+    def __radd__(self, other: datetime | TimestampSeries) -> TimestampSeries: ...  # type: ignore[override] # pyright: ignore[reportIncompatibleMethodOverride]


put back Timestamp

Dr-Irv · 2023-12-26T15:18:11Z

pandas-stubs/core/series.pyi

    def __mul__(  # type: ignore[override] # pyright: ignore[reportIncompatibleMethodOverride]
        self, other: num | Sequence[num] | Series[int] | Series[float]
    ) -> TimedeltaSeries: ...
    def __sub__(  # type: ignore[override] # pyright: ignore[reportIncompatibleMethodOverride]
-        self, other: Timedelta | TimedeltaSeries | TimedeltaIndex | np.timedelta64
+        self, other: timedelta | TimedeltaSeries | TimedeltaIndex | np.timedelta64


put back Timedelta

twoertwein · 2023-12-26T15:33:14Z

I'm working on the test and re-adding the redundancy. I might have a few intermediate commits.

About the redundancy: I personally would not add them as it is inconsistent with the best practices of int | float | complex, it is unnecessarily complex/verbose, and it might even be misleading as it implies that they are not sub-classes.

Dr-Irv · 2023-12-26T16:35:02Z

I'm working on the test and re-adding the redundancy. I might have a few intermediate commits.

OK - ping me when I should review it next.

About the redundancy: I personally would not add them as it is inconsistent with the best practices of int | float | complex, it is unnecessarily complex/verbose, and it might even be misleading as it implies that they are not sub-classes.

We debated this before. See the discussion at #341 (comment)

Another possibility is to define some internal types for typing purposes:

TimedeltaArg: TypeAlias = pd.Timedelta | datetime.timedelta | np.timedelta64
TimestampArg: TypeAlias = pd.Timestamp | datetime.datetime | np.datetime64

Then use them as arguments throughout. To avoid any issue with regards to subclass confusion, we could put comments above those aliases to indicate why we document it that way.

twoertwein · 2023-12-26T17:10:36Z

TimedeltaArg: TypeAlias = pd.Timedelta | datetime.timedelta | np.timedelta64
TimestampArg: TypeAlias = pd.Timestamp | datetime.datetime | np.datetime64

I like this idea (but not for this PR), assuming that the numpy types are accepted everywhere (I'm not sure - I believe they are not actual sub-classes)

twoertwein · 2023-12-26T17:11:23Z

I think I added all the test and all the redundant unions

Dr-Irv

Thanks @twoertwein . Not sure if we caught all the tests, but this will do for now.

Dr-Irv · 2023-12-26T17:19:05Z

I like this idea (but not for this PR), assuming that the numpy types are accepted everywhere (I'm not sure - I believe they are not actual sub-classes)

Yes, that is the part I'm not sure about either.

Timestamp -> datetime and Timedelta -> timedelta

f7a5a3a

Dr-Irv reviewed Dec 26, 2023

View reviewed changes

twoertwein added 2 commits December 26, 2023 09:56

Merge remote-tracking branch 'upstream/main' into time

5e4f959

keep redundancy

2a047e3

Dr-Irv requested changes Dec 26, 2023

View reviewed changes

twoertwein added 2 commits December 26, 2023 11:53

test

5d3144d

redundancy

e53850b

Dr-Irv approved these changes Dec 26, 2023

View reviewed changes

Dr-Irv merged commit 52384d3 into pandas-dev:main Dec 26, 2023
13 checks passed

twoertwein deleted the time branch February 10, 2024 20:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Timestamp -> datetime and Timedelta -> timedelta #841

Timestamp -> datetime and Timedelta -> timedelta #841

twoertwein commented Dec 24, 2023

Dr-Irv left a comment

Dr-Irv left a comment

Dr-Irv Dec 26, 2023

Dr-Irv Dec 26, 2023

Dr-Irv Dec 26, 2023

Dr-Irv Dec 26, 2023

Dr-Irv Dec 26, 2023

Dr-Irv Dec 26, 2023

Dr-Irv Dec 26, 2023

Dr-Irv Dec 26, 2023

Dr-Irv Dec 26, 2023

Dr-Irv Dec 26, 2023

twoertwein commented Dec 26, 2023

Dr-Irv commented Dec 26, 2023

twoertwein commented Dec 26, 2023

twoertwein commented Dec 26, 2023

Dr-Irv left a comment

Dr-Irv commented Dec 26, 2023

		@classmethod
		def fromtimestamp(cls, t: float, tz: _tzinfo \| str \| None = ...) -> Self: ...

Timestamp -> datetime and Timedelta -> timedelta #841

Timestamp -> datetime and Timedelta -> timedelta #841

Conversation

twoertwein commented Dec 24, 2023

Dr-Irv left a comment

Choose a reason for hiding this comment

Dr-Irv left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

twoertwein commented Dec 26, 2023

Dr-Irv commented Dec 26, 2023

twoertwein commented Dec 26, 2023

twoertwein commented Dec 26, 2023

Dr-Irv left a comment

Choose a reason for hiding this comment

Dr-Irv commented Dec 26, 2023