Skip to content

API: Add lib.array_utils namespace #24540

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.

Sign up for GitHub

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

Jump to bottom
Merged
ngoldbaum merged 7 commits into from
Sep 11, 2023
Merged

API: Add lib.array_utils namespace #24540

ngoldbaum merged 7 commits into from
Sep 11, 2023

Conversation

@mtsokol
Copy link
Member

@mtsokol mtsokol commented Aug 25, 2023

Relevant issues #24507 and #24166

Hi @rgommers @ngoldbaum,

This PR adds lib.array_utils module as as public namespace which creates only local namespace.
Initially it hosts three functions:

  • byte_bounds (moved from np.lib.utils)
  • normalize_axis_tuple
  • normalize_axis_index

This was referenced Aug 25, 2023
Closed
Closed
rgommers
rgommers reviewed Aug 25, 2023
View reviewed changes
Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

Thanks @mtsokol. This looks pretty good to me. One idea to consider: would it make sense to move what is now in array_utils.py to _array_utils_impl.py and make the content of a new array_utils.py only this:

from ._array_utils_impl import (
    byte_bounds,
    normalize_axis_index,
    normalize_axis_tuple,
)

That would make it pretty much bullet-proof again more accidental leaking in of stray objects.

@charris charris changed the title API: Add lib.array_utils namespace API: Add lib.array_utils namespace Aug 25, 2023
@rgommers
Copy link
Member

rgommers commented Aug 25, 2023

Also, as we just discussed, these are the first time we expose normalize_axis_index and normalize_axis_tuple as public API, so they should show up in the html docs after this PR.

@rgommers rgommers added this to the 2.0.0 release milestone Aug 25, 2023
@mtsokol mtsokol force-pushed the lib-arrayutils-namespace branch 4 times, most recently from 1fe19c6 to 2a6cec2 Compare August 26, 2023 16:50
@mtsokol
Copy link
Member Author

mtsokol commented Aug 28, 2023

Also, as we just discussed, these are the first time we expose normalize_axis_index and normalize_axis_tuple as public API, so they should show up in the html docs after this PR.

@rgommers Sure, I added these two functions to routines.other.rst file.

@rgommers
Copy link
Member

rgommers commented Aug 28, 2023

Two questions:

@mtsokol mtsokol mentioned this pull request Aug 28, 2023
Merged
@mtsokol
Copy link
Member Author

mtsokol commented Aug 28, 2023 

* Can you verify that the removal of `np.byte_bounds` is okay with `joblib` now?

It looks like joblib uses np.byte_bounds in two places and one np.NaN that has already been removed. I will created a PR to address it: joblib/joblib#1501

* Since this is the first new namespace, it may be useful to ping the mailing list and point them at [DISCUSS: Namespace for array creation/inspection utils? #24166](https://github.com/numpy/numpy/issues/24166) regarding the `array_utils` name. Can you do that?

Sure! I sent a message to the mailing list.

@mtsokol mtsokol self-assigned this Aug 29, 2023
@mtsokol mtsokol force-pushed the lib-arrayutils-namespace branch from 64bfe77 to 3cb96b6 Compare August 30, 2023 09:52
@ngoldbaum
Copy link
Member

ngoldbaum commented Aug 30, 2023

Given this adds a new public name to the API let's let this sit for a few more days to gather comments.

@mtsokol mtsokol force-pushed the lib-arrayutils-namespace branch from 3cb96b6 to 5fe1e1c Compare September 1, 2023 10:49
@mtsokol mtsokol force-pushed the lib-arrayutils-namespace branch from 5fe1e1c to 4dd54b5 Compare September 6, 2023 08:31
BvB93
BvB93 reviewed Sep 6, 2023
View reviewed changes
seberg
seberg reviewed Sep 6, 2023
View reviewed changes
rgommers
rgommers approved these changes Sep 9, 2023
View reviewed changes
Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

LGTM now. Everyone's comments have been addressed, and the email to the mailing list didn't get replies. So I think we're good here. I suggest we merge this in a day or two unless there are new comments.

@ngoldbaum
Copy link
Member

ngoldbaum commented Sep 11, 2023

I suggest we merge this in a day or two unless there are new comments.

In it goes. Thanks @mtsokol.

@ngoldbaum ngoldbaum merged commit 4f84d71 into numpy:main Sep 11, 2023
@mtsokol mtsokol deleted the lib-arrayutils-namespace branch September 11, 2023 15:49
BvB93 added a commit to BvB93/numpy that referenced this pull request Dec 21, 2023
@BvB93
TYP: Explicitly re-export np.lib.array_utils
d89f176 
xref numpy#24540
@brendan-m-murphy brendan-m-murphy mentioned this pull request Nov 11, 2024
Closed
11 tasks
@ntessore
Copy link
Contributor

ntessore commented Nov 21, 2024

Adding here that the .. versionadded:: 1.13.0 in the normalize_axis_* functions took me a while to figure out, since np.lib.array_utils didn't exist until 2.0 🙃

@seberg
Copy link
Member

seberg commented Nov 21, 2024

FWIW, the versionadded 1.13.0 are now deleted because it is uninterestingly ancient history anyway ;).

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

Reviewers

@seberg seberg seberg left review comments

@BvB93 BvB93 BvB93 left review comments

@rgommers rgommers rgommers approved these changes

Assignees

@mtsokol mtsokol

Labels

30 - API Numpy 2.0 API Changes

Projects

None yet

Milestone

2.0.0 release

Development

Successfully merging this pull request may close these issues.

6 participants

@mtsokol @rgommers @ngoldbaum @ntessore @seberg @BvB93