-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
TYP: timestamps.pyi #44339
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
TYP: timestamps.pyi #44339
Conversation
def combine( | ||
cls, date: _date, time: _time, tzinfo: _tzinfo | None = ... | ||
) -> datetime: ... | ||
def combine(cls, date: _date, time: _time) -> datetime: ... # type: ignore[override] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the implementation does not accept tzinfo
fold: int = ..., | ||
) -> datetime: ... | ||
def astimezone(self: _S, tz: _tzinfo | None = ...) -> _S: ... | ||
def ctime(self) -> str: ... | ||
def isoformat(self, sep: str = ..., timespec: str = ...) -> str: ... | ||
# error: Signature of "isoformat" incompatible with supertype "datetime" | ||
def isoformat(self, sep: str = ...) -> str: ... # type: ignore[override] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the implementation does not accept timespec
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm i thought we recently changed this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that seems to have changed since Nov 6 :)
I will look through the other cases that don't match datetime and correct them if necessary in a new PR.
def __add__(self, other: np.ndarray) -> np.ndarray: ... | ||
@overload | ||
# TODO: other can also be Tick (but it cannot be resolved) | ||
def __add__(self: _S, other: timedelta | np.timedelta64) -> _S: ... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
once offset.pyi from #43744 is in a mergeable from, Tick can be added.
pandas/_libs/tslibs/timestamps.pyi
Outdated
@@ -44,7 +44,7 @@ class Timestamp(datetime): | |||
| _date | |||
| datetime | |||
| np.datetime64 = ..., | |||
freq=..., | |||
freq: int | None = ..., |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think freq can be a DateOffset?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I will add that.
One question: DateOffset
is used in a few places, but I think no class inherits from it. There is even this case:
cdef class RelativeDeltaOffset(BaseOffset):
"""
DateOffset subclass backed by a dateutil relativedelta object. [...]
"""
Is DateOffset
still being used or should most of its uses be replaced with BaseOffset
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you're right, i think BaseOffset is technically correct. its annoying that DateOffset is a more informative name
This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comment but let's merge
fold: int = ..., | ||
) -> datetime: ... | ||
def astimezone(self: _S, tz: _tzinfo | None = ...) -> _S: ... | ||
def ctime(self) -> str: ... | ||
def isoformat(self, sep: str = ..., timespec: str = ...) -> str: ... | ||
# error: Signature of "isoformat" incompatible with supertype "datetime" | ||
def isoformat(self, sep: str = ...) -> str: ... # type: ignore[override] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm i thought we recently changed this
maybe worth it to use something more descriptive than |
IIRC thats what they used in the microsoft stubs that i based this on |
Should type annotations from a pyi files also be added to the pyx file (if possible)?