Skip to content
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

ENH: Add annotations for np.lib.twodim_base #19261

Merged
merged 2 commits into from
Jun 17, 2021
Merged

ENH: Add annotations for np.lib.twodim_base #19261

merged 2 commits into from
Jun 17, 2021

Conversation

BvB93
Copy link
Member

@BvB93 BvB93 commented Jun 16, 2021

This PR adds annotations for the 15 functions in np.lib.twodim_base, including some basic dtype support.

Comment on lines +235 to +239
def tril_indices(
n: int,
k: int = ...,
m: None | int = ...,
) -> Tuple[NDArray[int_], NDArray[int_]]: ...
Copy link
Member Author

Choose a reason for hiding this comment

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

We currently have four functions here that return indices-arrays of the np.int_ dtype, rather than np.intp.
Would this be worthwhile to change?

Copy link
Member

Choose a reason for hiding this comment

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

Certainly worth discussing, I doubt it would cause problems. I'd like to get rid of C long, it is too variable.

Comment on lines +219 to +227
# NOTE: we're assuming/demanding here the `mask_func` returns
# an ndarray of shape `(n, n)`; otherwise there is the possibility
# of the output tuple having more or less than 2 elements
@overload
def mask_indices(
n: int,
mask_func: _MaskFunc[int],
k: int = ...,
) -> Tuple[NDArray[intp], NDArray[intp]]: ...
Copy link
Member Author

Choose a reason for hiding this comment

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

_MaskFunc is currently expected to return a 2D array (shape (n, n)).
I'd say this is a reasonable demand, even though it is a bit stricter than is truly necessary.

@charris charris merged commit 1e2aa70 into numpy:main Jun 17, 2021
@charris
Copy link
Member

charris commented Jun 17, 2021

Thanks Bas.

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.

None yet

2 participants