Skip to content

ENH: Added missing methods to np.flatiter#17180

Merged
mattip merged 7 commits intonumpy:masterfrom
BvB93:flatiter
Sep 2, 2020
Merged

ENH: Added missing methods to np.flatiter#17180
mattip merged 7 commits intonumpy:masterfrom
BvB93:flatiter

Conversation

@BvB93
Copy link
Copy Markdown
Member

@BvB93 BvB93 commented Aug 28, 2020

Closes #17114 and #17113.

This pull requests adds annotations to the following three np.flatiter methods:

  • __getitem__()
  • __len__()
  • __array__()

@BvB93
Copy link
Copy Markdown
Member Author

BvB93 commented Aug 28, 2020

Pinging @bashtage.

a.index = Any # E: Property "index" defined in "flatiter" is read-only
a.copy(order='C') # E: Unexpected keyword argument
a[np.bool_()] # E: No overload variant of "__getitem__"
a[Index()] # E: No overload variant of "__getitem__"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be legal? Might be worth referencing an issue if its not.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be, but the behavior of flatiter.__getitem__ is very much inconsistent
with respect to ndarray.__getitem__ (case in point: #17175).

Adding a note is probably a good idea though (see edcf0ea).

Contrary to `ndarray.__getitem__` its counterpart in `flatiter` does not accept objects with the `__array__` or `__index__` protocols;
boolean indexing is just plain broken (numpygh-17175)
def __setitem__(self, key: str, value: bool) -> None: ...

_ArrayLikeInt = Union[
int,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does [int, integer] include bool?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does as bool is a subclass of int.

There is, as far as I'm aware, no convenient way of annotating a function which accepts integers but rejects booleans.

Copy link
Copy Markdown
Member

@eric-wieser eric-wieser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks safe enough

@mattip mattip merged commit 5b37a4b into numpy:master Sep 2, 2020
@mattip
Copy link
Copy Markdown
Member

mattip commented Sep 2, 2020

Thanks @BvB93

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ENH: flatiter is missing _SupportsArray in type definition ENH: Add __getitem__ to flatiter type definition

4 participants