-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
TYP: Use Self instead of class-bound TypeVar (generic.py/frame.py/series.py) #51493
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
Conversation
5089ea8
to
089920e
Compare
089920e
to
86b5fad
Compare
pandas/core/indexes/base.py
Outdated
@@ -357,7 +354,7 @@ class Index(IndexOpsMixin, PandasObject): | |||
# given the dtypes of the passed arguments | |||
|
|||
@final | |||
def _left_indexer_unique(self: _IndexT, other: _IndexT) -> npt.NDArray[np.intp]: | |||
def _left_indexer_unique(self, other: Index) -> npt.NDArray[np.intp]: |
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.
Is this change intended? Is _left_indexer_unique(self, other: Self)
to strict?
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.
(same for the following methods)
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.
Yeah, I was in doubt about this too. Fir the built-in index classes I think it will always require same class (so other: Self
), but custom user-created classes:
>>> MyIndex(Index):
... pass
In this case, MyIndex
has same dtype as the base index, so should be comparable to Index
in this method? (I'm leaning to yes; that this should be possible, but I'm not 100% sure).
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.
For the built-in index classes I think it will always require same class (so
other: Self
)
I would then stick with Self
it is also stricter which could help catch bugs (or causes a false mypy error which can then be fixed by someone who knows for sure).
Thanks @topper-123! |
Progress towards #51233.
There were more updates to do than I expected, so I split it up a bit. In this PR I do
pandas/core/generic.py
,pandas/core/frame.py
,pandas/core/series.py
&pandas/indexes/
.