-
-
Notifications
You must be signed in to change notification settings - Fork 10.9k
MAINT: simplifying annotations for np.core.from_numeric #16556
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
Conversation
* Affects a set of <20 functions from ``np.core.fromnumeric``. * Based on feedback from numpy/numpy-stubs#71.
* Continuation of 09c2204.
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.
LGTM modulo test failures.
The tests have been fixed as of 0f7f6a9. If there are no further comments then the pull request can be merged, as far as I'm concerned. |
Thanks @BvB93! |
) -> Union[integer, bool_]: ... | ||
@overload | ||
def choose( | ||
a: _ArrayLikeIntOrBool, | ||
choices: Union[Sequence[ArrayLike], ndarray], | ||
choices: ArrayLike, |
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.
choices: ArrayLike, | |
choices: Sequence[ArrayLike], |
I think it's important to emphasize that this input is not an array, as it means shapes like (0, 3)
are not handled sensibly.
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.
Same for the other overload
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.
The tricky part is that, without proper shape support, expressing this will be either very difficult and/or verbose.
For example even Sequence[ArrayLike]
does not include the likes of np.ndarray
or pd.DataFrane
, both of which are (strictly speaking) not proper sequences.
In one of the last pull requests over at numpy-stubs (numpy/numpy-stubs#71) we had a discussion about the complexity of the pull requests' respective annotations, more specifically about the ones trying to express >0D array-like objects (vectors, matrices, etc.).
As the complexity of the annotations came at the detriment of their readability, we arrived at the conclusion that it would be best to simplify them in a future pull request.
This pull request thus addresses aforementioned issue:
ArrayLike
.See Also