TYP: misc typing in core\indexes\base.py #35991
Conversation
pandas/core/indexes/base.py
Outdated
|
|
||
|
|
||
| str_ = str |
There was a problem hiding this comment.
i think elsewhere we use "str_t" for this
There was a problem hiding this comment.
we use both str_t and str_type so why no have another?
only jesting. see #32365 (comment)
will change to str_t for now.
| @@ -5516,7 +5534,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.
does Sequence not subsume ArrayLike?
| def slice_indexer( | ||
| self, | ||
| start: Optional[Label] = None, | ||
| end: Optional[Label] = None, |
There was a problem hiding this comment.
isnt Optional redundant here?
There was a problem hiding this comment.
we've got no_implicit_optional=True in out setup.cfg (but I don't know why?)
it would work, but only since we have Label = Optional[Hashable] to allow None to be a Label. Label would be easier to grok if it were Union[Hashable, None] (or if Hashable included None) but for consistency we use Optional.
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.
| @@ -188,6 +190,9 @@ def _new_Index(cls, d): | |||
| return cls.__new__(cls, **d) | |||
|
|
|||
|
|
|||
| _IndexT = TypeVar("_IndexT", bound="Index") | |||
There was a problem hiding this comment.
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.
TL;DR lmk explictly what you want to name the TypeVar otherwise i'll leave it as is for now pending further discussion.
also pls confirm that the ref is consistent
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 of T for TypeVars used to preserve return types or we could drop the T 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 (so FrameOrSeries could be NDFrameSubClass)
e.g. FrameOrSeries is different
FrameOrSeries was originally
FrameOrSeries = TypeVar("FrameOrSeries", "Series", "DataFrame")
before being changed in #28173 to
FrameOrSeries = TypeVar("FrameOrSeries", bound="NDFrame")
can you add a comment before this (and in other cases where we use TypeVar like this)
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/
Fundamental building blocks:
- Any, used as def get(key: str) -> Any: ...
- Union, used as Union[Type1, Type2, Type3]
- Callable, used as Callable[[Arg1Type, Arg2Type], ReturnType]
- Tuple, used by listing the element types, for example Tuple[int, int, str]. The empty tuple can be typed as Tuple[()]. Arbitrary-length homogeneous tuples can be expressed using one type and ellipsis, for example Tuple[int, ...]. (The ... here are part of the syntax, a literal ellipsis.)
- TypeVar, used as X = TypeVar('X', Type1, Type2, Type3) or simply Y = TypeVar('Y') (see above for more details)
- Generic, used to create user-defined generic classes
- Type, used to annotate class objects
There was a problem hiding this comment.
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.
obviously can do this in a dedicated PR
|
thanks @simonjayhawkins |
No description provided.