-
Notifications
You must be signed in to change notification settings - Fork 123
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
feat: add Series.dtype
return annotation
#1049
Conversation
Series.dtype
return annotation
@@ -302,7 +303,7 @@ def len(self) -> int: | |||
return len(self._compliant_series) | |||
|
|||
@property | |||
def dtype(self) -> Any: | |||
def dtype(self: Self) -> DType: |
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.
Thank you!
@@ -359,7 +359,7 @@ def alias(self, name: str) -> Self: | |||
) | |||
|
|||
@property | |||
def dtype(self) -> DType: | |||
def dtype(self: Self) -> DType: |
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 don't believe it's needed
def dtype(self: Self) -> DType: | |
def dtype(self) -> DType: |
But if it is... Can you please teach me why? Thank you
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 took this as opportunity to go a bit banana into hints for the rest of the codebase (uncommitted here). When we know that self: Self
, then self._native_series
is known to be a pa.ChunkedArray
, which has a type
property.
Probably some editors do not need it anyway π
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.
sounds like something which editors should fix π but in the meantime, if this helps the development workflow, then it seems like a good idea to add it π
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.
thanks @FBruzzesi !
What type of PR is this? (check all applicable)
Related issues
nw.DType
fromSeries
's.dtype
propertyΒ #1047Checklist
If you have comments or can explain your changes, please do so below.