-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
TYP: misc typing in core\indexes\base.py #35991
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
Changes from all commits
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 |
---|---|---|
|
@@ -10,6 +10,8 @@ | |
Hashable, | ||
List, | ||
Optional, | ||
Sequence, | ||
TypeVar, | ||
Union, | ||
) | ||
import warnings | ||
|
@@ -22,7 +24,7 @@ | |
from pandas._libs.tslibs import OutOfBoundsDatetime, Timestamp | ||
from pandas._libs.tslibs.period import IncompatibleFrequency | ||
from pandas._libs.tslibs.timezones import tz_compare | ||
from pandas._typing import DtypeObj, Label | ||
from pandas._typing import AnyArrayLike, Dtype, DtypeObj, Label | ||
from pandas.compat import set_function_name | ||
from pandas.compat.numpy import function as nv | ||
from pandas.errors import InvalidIndexError | ||
|
@@ -98,7 +100,7 @@ | |
) | ||
|
||
if TYPE_CHECKING: | ||
from pandas import Series | ||
from pandas import RangeIndex, Series | ||
|
||
|
||
__all__ = ["Index"] | ||
|
@@ -188,6 +190,9 @@ def _new_Index(cls, d): | |
return cls.__new__(cls, **d) | ||
|
||
|
||
_IndexT = TypeVar("_IndexT", bound="Index") | ||
|
||
|
||
class Index(IndexOpsMixin, PandasObject): | ||
""" | ||
Immutable ndarray implementing an ordered, sliceable set. The basic object | ||
|
@@ -787,7 +792,13 @@ def repeat(self, repeats, axis=None): | |
# -------------------------------------------------------------------- | ||
# Copying Methods | ||
|
||
def copy(self, name=None, deep=False, dtype=None, names=None): | ||
def copy( | ||
self: _IndexT, | ||
name: Optional[Label] = None, | ||
deep: bool = False, | ||
dtype: Optional[Dtype] = None, | ||
names: Optional[Sequence[Label]] = None, | ||
) -> _IndexT: | ||
""" | ||
Make a copy of this object. | ||
|
||
|
@@ -949,10 +960,9 @@ def _format_with_header( | |
# could have nans | ||
mask = isna(values) | ||
if mask.any(): | ||
result = np.array(result) | ||
result[mask] = na_rep | ||
# error: "List[str]" has no attribute "tolist" | ||
result = result.tolist() # type: ignore[attr-defined] | ||
result_arr = np.array(result) | ||
result_arr[mask] = na_rep | ||
result = result_arr.tolist() | ||
else: | ||
result = trim_front(format_array(values, None, justify="left")) | ||
return header + result | ||
|
@@ -4913,7 +4923,13 @@ def _get_string_slice(self, key: str_t, use_lhs: bool = True, use_rhs: bool = Tr | |
# overridden in DatetimeIndex, TimedeltaIndex and PeriodIndex | ||
raise NotImplementedError | ||
|
||
def slice_indexer(self, start=None, end=None, step=None, kind=None): | ||
def slice_indexer( | ||
self, | ||
start: Optional[Label] = None, | ||
end: Optional[Label] = None, | ||
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. isnt Optional redundant here? 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. we've got it would work, but only since we have Label = Optional[Hashable] to allow Here, for consistency of keyword parameter annotations, don't really want to remove the Optional, even though already accounted for in Label. but I'll change if blocker. |
||
step: Optional[int] = None, | ||
kind: Optional[str_t] = None, | ||
) -> slice: | ||
""" | ||
Compute the slice indexer for input labels and step. | ||
|
||
|
@@ -5513,7 +5529,9 @@ def ensure_index_from_sequences(sequences, names=None): | |
return MultiIndex.from_arrays(sequences, names=names) | ||
|
||
|
||
def ensure_index(index_like, copy: bool = False): | ||
def ensure_index( | ||
index_like: Union[AnyArrayLike, Sequence], copy: bool = False | ||
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. does Sequence not subsume ArrayLike? 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. see #28770 |
||
) -> Index: | ||
""" | ||
Ensure that we have an index from some index-like object. | ||
|
||
|
@@ -5549,7 +5567,18 @@ def ensure_index(index_like, copy: bool = False): | |
index_like = index_like.copy() | ||
return index_like | ||
if hasattr(index_like, "name"): | ||
return Index(index_like, name=index_like.name, copy=copy) | ||
# https://github.com/python/mypy/issues/1424 | ||
# error: Item "ExtensionArray" of "Union[ExtensionArray, | ||
# Sequence[Any]]" has no attribute "name" [union-attr] | ||
# error: Item "Sequence[Any]" of "Union[ExtensionArray, Sequence[Any]]" | ||
# has no attribute "name" [union-attr] | ||
# error: "Sequence[Any]" has no attribute "name" [attr-defined] | ||
# error: Item "Sequence[Any]" of "Union[Series, Sequence[Any]]" has no | ||
# attribute "name" [union-attr] | ||
# error: Item "Sequence[Any]" of "Union[Any, Sequence[Any]]" has no | ||
# attribute "name" [union-attr] | ||
name = index_like.name # type: ignore[union-attr, attr-defined] | ||
return Index(index_like, name=name, copy=copy) | ||
|
||
if is_iterator(index_like): | ||
index_like = list(index_like) | ||
|
@@ -5604,7 +5633,7 @@ def _validate_join_method(method: str): | |
raise ValueError(f"do not recognize join method {method}") | ||
|
||
|
||
def default_index(n): | ||
def default_index(n: int) -> "RangeIndex": | ||
from pandas.core.indexes.range import RangeIndex | ||
|
||
return RangeIndex(0, n, name=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.
can you add a comment before this (and in other cases where we use TypeVar like this); also pls confirm that the ref is consistent (e.g. FrameOrSeries is different )
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.
TL;DR lmk explictly what you want to name the TypeVar otherwise i'll leave it as is for now pending further discussion.
we are far from consistent, the use of TypeVar outside of pandas._typing is
_T = TypeVar("_T", bound="NDArrayBackedExtensionArray") added in #33660
DatetimeLikeArrayT = TypeVar("DatetimeLikeArrayT", bound="DatetimeLikeArrayMixin") added in #33706
BaseMaskedArrayT = TypeVar("BaseMaskedArrayT", bound="BaseMaskedArray") added in #31728
_T = TypeVar("_T", bound="BaseExprVisitor") added in #31365
ScalarResult = TypeVar("ScalarResult")
OutputFrameOrSeries = TypeVar("OutputFrameOrSeries", bound=NDFrame) added in #33286
_T = TypeVar("_T", bound="DatetimeIndexOpsMixin") added in #33839
T = TypeVar("T", bound="BlockManager") added in #32421
DatetimeScalar = TypeVar("DatetimeScalar", Scalar, datetime)
and a couple of uses of
_KT = TypeVar("_KT")
_VT = TypeVar("_VT")
my preference is
_<classname>T
, i.e. leading underscore followed by the class name passed as the bound argument followed by a uppercase T to indicate TypeVar. (of course where a union is used instead of a bound this allows for more imaginative naming)In pandas._typing, the TypeVars are imported by other modules, so we don't use leading underscores
see also https://github.com/numpy/numpy/blob/3fbc84a5662ffd985a071b0bbdcd59e655041ad3/numpy/__init__.pyi for other ideas on naming.
we could add
Self
suffix instead ofT
for TypeVars used to preserve return types or we could drop theT
altogether. so using that naming convention, we would change the_IndexT
added here to_IndexSelf
since this TypeVar is used to maintain the return type of.copy()
for abstract/base classes we could add
SubClass
suffix (soFrameOrSeries
could beNDFrameSubClass
)FrameOrSeries was originally
FrameOrSeries = TypeVar("FrameOrSeries", "Series", "DataFrame")
before being changed in #28173 to
FrameOrSeries = TypeVar("FrameOrSeries", bound="NDFrame")
TypeVar is a fundamental building block[1] of typing and if we are consistent with the naming, additional comments explaining fundamental use of typing shouldn't be necessary.
[1] from https://www.python.org/dev/peps/pep-0484/
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 as long as consistent doesn't matter much, IndexT looks good to me: importable (no leading _), not too crazy
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.
obviously can do this in a dedicated PR