-
-
Notifications
You must be signed in to change notification settings - Fork 18.6k
TYP: annotations in indexes #31181
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: annotations in indexes #31181
Changes from all commits
21ff7c6
172a923
e842524
9f8d03f
6235606
294f625
e352231
964b7b1
06fde05
9e5ca95
2252c9f
6015551
585f1fb
e49ebbd
2f54a48
b2f195b
ac64bb0
d16a3a3
4288cd2
2ab8e3e
b85bea3
87f5311
c15ca51
9f188ed
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,7 @@ | |
""" | ||
import builtins | ||
import textwrap | ||
from typing import Dict, FrozenSet, List, Optional | ||
from typing import Dict, FrozenSet, List, Optional, Union | ||
|
||
import numpy as np | ||
|
||
|
@@ -112,7 +112,7 @@ def _freeze(self): | |
object.__setattr__(self, "__frozen", True) | ||
|
||
# prevent adding any attribute via s.xxx.new_attribute = ... | ||
def __setattr__(self, key, value): | ||
def __setattr__(self, key: str, value): | ||
# _cache is used by a decorator | ||
# We need to check both 1.) cls.__dict__ and 2.) getattr(self, key) | ||
# because | ||
|
@@ -241,7 +241,7 @@ def __getitem__(self, key): | |
raise KeyError(f"Column not found: {key}") | ||
return self._gotitem(key, ndim=1) | ||
|
||
def _gotitem(self, key, ndim, subset=None): | ||
def _gotitem(self, key, ndim: int, subset=None): | ||
""" | ||
sub-classes to define | ||
return a sliced object | ||
|
@@ -607,6 +607,11 @@ class IndexOpsMixin: | |
["tolist"] # tolist is not deprecated, just suppressed in the __dir__ | ||
) | ||
|
||
@property | ||
def _values(self) -> Union[ExtensionArray, np.ndarray]: | ||
# must be defined here as a property for mypy | ||
raise AbstractMethodError(self) | ||
|
||
def transpose(self, *args, **kwargs): | ||
""" | ||
Return the transpose, which is by definition self. | ||
|
@@ -632,6 +637,10 @@ def shape(self): | |
""" | ||
return self._values.shape | ||
|
||
def __len__(self) -> int: | ||
# We need this defined here for mypy | ||
raise AbstractMethodError(self) | ||
|
||
@property | ||
def ndim(self) -> int: | ||
""" | ||
|
@@ -666,14 +675,14 @@ def item(self): | |
raise ValueError("can only convert an array of size 1 to a Python scalar") | ||
|
||
@property | ||
def nbytes(self): | ||
def nbytes(self) -> int: | ||
""" | ||
Return the number of bytes in the underlying data. | ||
""" | ||
return self._values.nbytes | ||
|
||
@property | ||
def size(self): | ||
def size(self) -> int: | ||
""" | ||
Return the number of elements in the underlying data. | ||
""" | ||
|
@@ -1079,7 +1088,14 @@ def hasnans(self): | |
return bool(isna(self).any()) | ||
|
||
def _reduce( | ||
self, op, name, axis=0, skipna=True, numeric_only=None, filter_type=None, **kwds | ||
self, | ||
op, | ||
name: str, | ||
axis=0, | ||
skipna=True, | ||
numeric_only=None, | ||
filter_type=None, | ||
**kwds, | ||
): | ||
""" perform the reduction type operation if we can """ | ||
func = getattr(self, name, None) | ||
|
@@ -1275,7 +1291,7 @@ def unique(self): | |
|
||
return result | ||
|
||
def nunique(self, dropna=True): | ||
def nunique(self, dropna: bool = True) -> int: | ||
""" | ||
Return number of unique elements in the object. | ||
|
||
|
@@ -1316,7 +1332,7 @@ def nunique(self, dropna=True): | |
return n | ||
|
||
@property | ||
def is_unique(self): | ||
def is_unique(self) -> bool: | ||
""" | ||
Return boolean if values in the object are unique. | ||
|
||
|
@@ -1327,7 +1343,7 @@ def is_unique(self): | |
return self.nunique(dropna=False) == len(self) | ||
|
||
@property | ||
def is_monotonic(self): | ||
def is_monotonic(self) -> bool: | ||
""" | ||
Return boolean if values in the object are | ||
monotonic_increasing. | ||
|
@@ -1340,7 +1356,11 @@ def is_monotonic(self): | |
|
||
return Index(self).is_monotonic | ||
|
||
is_monotonic_increasing = is_monotonic | ||
@property | ||
def is_monotonic_increasing(self) -> bool: | ||
"""alias for is_monotonic""" | ||
# mypy complains if we alias directly | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. probably don't need the comment. In Index, Although without going deeper, looks strange that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
it does seem a little weird, but its because this gets mixed in to Series, so Series.is_monotonic needs to wrap in Index so that it can use IndexEngine |
||
return self.is_monotonic | ||
|
||
@property | ||
def is_monotonic_decreasing(self) -> bool: | ||
|
@@ -1493,7 +1513,7 @@ def factorize(self, sort=False, na_sentinel=-1): | |
|
||
@Substitution(klass="Index") | ||
@Appender(_shared_docs["searchsorted"]) | ||
def searchsorted(self, value, side="left", sorter=None): | ||
def searchsorted(self, value, side="left", sorter=None) -> np.ndarray: | ||
return algorithms.searchsorted(self._values, value, side=side, sorter=sorter) | ||
|
||
def drop_duplicates(self, keep="first", inplace=False): | ||
|
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.
What error was this throwing? Slightly hesitant to do something like this as it could interfere with Mixins that define this otherwise
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.
pandas/core/base.py:1343: error: Argument 1 to "len" has incompatible type "IndexOpsMixin"; expected "Sized"
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.
It might make sense to just ignore that line; I think mypy generally doesn't play nice with Mixins so may be a few things like this, but I would imagine the intent is for that to be defined by whatever gets composed with this class
Not a big deal for me either way maybe just a note for other reviewers. Otherwise this lgtm
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.
@simonjayhawkins thoughts here?
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.
perhaps for consistency with addition for
_values
above,__len__
could raiseAbstractMethodError
instead?another alternative could be to add a type declaration at the top of the class instead,
__len__: Callable[..., int]
or ignore as @WillAyd suggested.