DOC/TYP: index.take return val #40521

mzeitlin11 · 2021-03-19T18:52:21Z

mzeitlin11 · 2021-03-19T18:54:00Z

pandas/core/indexes/base.py

+        allow_fill: bool = True,
+        fill_value=None,
+        **kwargs,
+    ):


Any advice on typing this return value? Can almost use the TypeVar _IndexT, but the problem is that RangeIndex.take returns a superclass (Int64Index).

I think in the base class use Index, and can then use a typevar in the subclasses that do return the same type.

I tried something along those lines originally - the problem I was running into is how to handle subclasses which rely on parent implementations. For example, for ExtensionIndex would like to be able to write a stub like

def take( self: _ExtensionIndexT, indices, axis: int = 0, allow_fill: bool = True, fill_value=None, **kwargs, ) -> _ExtensionIndexT: ...

to essentially type the child while still relying on the inherited implementation. But couldn't figure out a way to achieve this.

jbrockmendel · 2021-03-22T04:03:10Z

pandas/core/indexes/base.py

-    def take(self, indices, axis=0, allow_fill=True, fill_value=None, **kwargs):
+    def take(
+        self,
+        indices: Union[ArrayLike, Sequence[int]],


is ArrayLike really needed here? it should also be a Sequence[int]

see #28770 for issue about EA, but applicable to Series, Index and np.ndarray also

from typing import Sequence, cast import numpy as np import pandas as pd from pandas.core.arrays import ExtensionArray df = pd.DataFrame({"a": [1, 2, 3]}) idx = df.index arr: np.ndarray = cast(np.ndarray, idx._values) ea: ExtensionArray = pd.array(arr, dtype="Int64") ser = pd.Series(arr) reveal_type(idx) reveal_type(arr) reveal_type(ea) reveal_type(ser) def func(arr: Sequence): pass func(idx) func(arr) func(ea) func(ser)

/home/simon/t.py:11: note: Revealed type is 'pandas.core.indexes.base.Index' /home/simon/t.py:12: note: Revealed type is 'numpy.ndarray' /home/simon/t.py:13: note: Revealed type is 'pandas.core.arrays.base.ExtensionArray' /home/simon/t.py:14: note: Revealed type is 'pandas.core.series.Series' /home/simon/t.py:21: error: Argument 1 to "func" has incompatible type "Index"; expected "Sequence[Any]" [arg-type] /home/simon/t.py:22: error: Argument 1 to "func" has incompatible type "ndarray"; expected "Sequence[Any]" [arg-type] /home/simon/t.py:23: error: Argument 1 to "func" has incompatible type "ExtensionArray"; expected "Sequence[Any]" [arg-type] /home/simon/t.py:24: error: Argument 1 to "func" has incompatible type "Series"; expected "Sequence[Any]" [arg-type]

simonjayhawkins

Thanks @mzeitlin11 for the PR. generally lgtm, a couple of suggestions.

pandas/core/indexes/base.py

simonjayhawkins · 2021-03-22T10:45:36Z

pandas/core/indexes/base.py

-    def take(self, indices, axis=0, allow_fill=True, fill_value=None, **kwargs):
+    def take(
+        self,
+        indices: Union[ArrayLike, Sequence[int]],


see #28770 for issue about EA, but applicable to Series, Index and np.ndarray also

from typing import Sequence, cast import numpy as np import pandas as pd from pandas.core.arrays import ExtensionArray df = pd.DataFrame({"a": [1, 2, 3]}) idx = df.index arr: np.ndarray = cast(np.ndarray, idx._values) ea: ExtensionArray = pd.array(arr, dtype="Int64") ser = pd.Series(arr) reveal_type(idx) reveal_type(arr) reveal_type(ea) reveal_type(ser) def func(arr: Sequence): pass func(idx) func(arr) func(ea) func(ser)

/home/simon/t.py:11: note: Revealed type is 'pandas.core.indexes.base.Index' /home/simon/t.py:12: note: Revealed type is 'numpy.ndarray' /home/simon/t.py:13: note: Revealed type is 'pandas.core.arrays.base.ExtensionArray' /home/simon/t.py:14: note: Revealed type is 'pandas.core.series.Series' /home/simon/t.py:21: error: Argument 1 to "func" has incompatible type "Index"; expected "Sequence[Any]" [arg-type] /home/simon/t.py:22: error: Argument 1 to "func" has incompatible type "ndarray"; expected "Sequence[Any]" [arg-type] /home/simon/t.py:23: error: Argument 1 to "func" has incompatible type "ExtensionArray"; expected "Sequence[Any]" [arg-type] /home/simon/t.py:24: error: Argument 1 to "func" has incompatible type "Series"; expected "Sequence[Any]" [arg-type]

simonjayhawkins · 2021-03-22T10:47:27Z

pandas/core/indexes/base.py

+        allow_fill: bool = True,
+        fill_value=None,
+        **kwargs,
+    ):


I think in the base class use Index, and can then use a typevar in the subclasses that do return the same type.

Co-authored-by: Simon Hawkins <simonjayhawkins@gmail.com>

…nto doc/index_take_meth

jbrockmendel · 2021-03-28T16:31:10Z

pandas/core/indexes/base.py

-    def take(self, indices, axis=0, allow_fill=True, fill_value=None, **kwargs):
+    def take(
+        self,
+        indices: Union[AnyArrayLike, Sequence[int]],


isnt Sequence[int] going to cover the AnyArrayLike case?

I think even with AnyArrayLike in place of ArrayLike we run into the issue mentioned here #40521 (comment)

this is too complicated and specific an annotation to put here, leave this untyped for now, or put it in a typing alias, but then we should enforce it, alt we shouldn't allow non-array-likes here

Left untyped for now, now mostly just a doc fix

jbrockmendel · 2021-03-28T16:31:34Z

pandas/core/indexes/base.py

@@ -886,7 +886,7 @@ def astype(self, dtype, copy=True):

        Parameters
        ----------
-        indices : list
+        indices : array-like


i think Sequence[int] would be more helpful

Happy to change, my only question would be if we want to be using type hints for user facing API. The numpy documentation for take just uses array_like

i think Sequence[int] would be more helpful

we shouldn't use typing notation for docstrings. https://pandas.pydata.org/pandas-docs/dev/development/contributing_docstring.html#parameter-types

and from that guide...

If the exact type is not relevant, but must be compatible with a NumPy array, array-like can be specified. If Any type that can be iterated is accepted, iterable can be used:

but the guide doesn't mention sequence.

maybe sequence of int is allowed, but I think matching numpy is fine.

jreback · 2021-04-02T17:27:01Z

pandas/core/indexes/base.py

-    def take(self, indices, axis=0, allow_fill=True, fill_value=None, **kwargs):
+    def take(
+        self,
+        indices: Union[AnyArrayLike, Sequence[int]],


this is too complicated and specific an annotation to put here, leave this untyped for now, or put it in a typing alias, but then we should enforce it, alt we shouldn't allow non-array-likes here

simonjayhawkins · 2021-05-22T16:00:52Z

@jbrockmendel can you take another look, @jreback comments have been addressed.

jbrockmendel

LGTM

simonjayhawkins · 2021-05-23T11:42:13Z

Thanks @mzeitlin11

mzeitlin11 added 2 commits March 19, 2021 14:27

DOC/TYP: index.take

5253d39

Fix default

67b6299

mzeitlin11 commented Mar 19, 2021

View reviewed changes

mzeitlin11 mentioned this pull request Mar 19, 2021

CLN/TYP: remove unused arguments in merge #40513

Merged

jreback added the Typing type annotations, mypy/pyright type checking label Mar 21, 2021

jreback requested a review from simonjayhawkins March 21, 2021 21:20

jbrockmendel reviewed Mar 22, 2021

View reviewed changes

simonjayhawkins reviewed Mar 22, 2021

View reviewed changes

simonjayhawkins added this to the 1.3 milestone Mar 22, 2021

mzeitlin11 and others added 4 commits March 22, 2021 13:04

Update pandas/core/indexes/base.py

b609767

Co-authored-by: Simon Hawkins <simonjayhawkins@gmail.com>

Use AnyArrayLike instead of ArrayLike

8b917de

Co-authored-by: Simon Hawkins <simonjayhawkins@gmail.com>

Merge remote-tracking branch 'origin/master' into doc/index_take_meth

eb6016a

Merge branch 'doc/index_take_meth' of github.com:/mzeitlin11/pandas i…

2ff3ea5

…nto doc/index_take_meth

jbrockmendel reviewed Mar 28, 2021

View reviewed changes

Merge remote-tracking branch 'origin/master' into doc/index_take_meth

a53cb32

jreback requested changes Apr 2, 2021

View reviewed changes

mzeitlin11 added 8 commits April 2, 2021 17:13

Merge remote-tracking branch 'origin/master' into doc/index_take_meth

d7ec12c

Remove complicated typing, doc fixes

c37be8c

Merge remote-tracking branch 'origin/master' into doc/index_take_meth

b23c43d

Merge remote-tracking branch 'origin/master' into doc/index_take_meth

a16b108

Merge remote-tracking branch 'origin/master' into doc/index_take_meth

74ca93a

Merge remote-tracking branch 'origin' into doc/index_take_meth

9c34ff3

Merge remote-tracking branch 'upstream/master' into doc/index_take_meth

abd8e7c

Merge remote-tracking branch 'upstream/master' into doc/index_take_meth

a14be0b

simonjayhawkins added the Index Related to the Index class or subclasses label May 22, 2021

jbrockmendel approved these changes May 22, 2021

View reviewed changes

simonjayhawkins added the Docs label May 23, 2021

simonjayhawkins merged commit 3457359 into pandas-dev:master May 23, 2021

TLouf pushed a commit to TLouf/pandas that referenced this pull request Jun 1, 2021

DOC/TYP: index.take return val (pandas-dev#40521)

083a64c

JulianWgs pushed a commit to JulianWgs/pandas that referenced this pull request Jul 3, 2021

DOC/TYP: index.take return val (pandas-dev#40521)

9ca8bc0

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DOC/TYP: index.take return val #40521

DOC/TYP: index.take return val #40521

mzeitlin11 commented Mar 19, 2021

mzeitlin11 Mar 19, 2021

simonjayhawkins Mar 22, 2021

mzeitlin11 Mar 22, 2021

jbrockmendel Mar 22, 2021

simonjayhawkins Mar 22, 2021

simonjayhawkins left a comment

simonjayhawkins Mar 22, 2021

simonjayhawkins Mar 22, 2021

jbrockmendel Mar 28, 2021

mzeitlin11 Mar 28, 2021

jreback Apr 2, 2021

mzeitlin11 Apr 2, 2021

jbrockmendel Mar 28, 2021

mzeitlin11 Mar 28, 2021

simonjayhawkins Apr 27, 2021

jreback Apr 2, 2021

simonjayhawkins commented May 22, 2021

jbrockmendel left a comment

simonjayhawkins commented May 23, 2021

DOC/TYP: index.take return val #40521

DOC/TYP: index.take return val #40521

Conversation

mzeitlin11 commented Mar 19, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simonjayhawkins left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simonjayhawkins commented May 22, 2021

jbrockmendel left a comment

Choose a reason for hiding this comment

simonjayhawkins commented May 23, 2021