FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

API: Add ``lib.array_utils`` namespace by mtsokol · Pull Request #24540 · numpy/numpy · GitHub

/ numpy Public

API: Add lib.array_utils namespace - #24540

Merged
ngoldbaum merged 7 commits into
numpy:mainfrom
mtsokol:lib-arrayutils-namespace
Sep 11, 2023
ConversationCommits (7)ChecksFiles changed
Merged

API: Add lib.array_utils namespace#24540
ngoldbaum merged 7 commits into
numpy:mainfrom
mtsokol:lib-arrayutils-namespace

Conversation

mtsokol commented Aug 25, 2023

Copy link
Copy Markdown
Member

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 reviewed Aug 25, 2023
View reviewed changes

rgommers left a comment

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

Comment thread numpy/typing/tests/data/fail/lib_utils.pyi
charris changed the title API: Add lib.array_utils namespace API: Add lib.array_utils namespace Aug 25, 2023

rgommers commented Aug 25, 2023

Copy link
Copy Markdown
Member

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 added this to the 2.0.0 release milestone Aug 25, 2023
mtsokol force-pushed the lib-arrayutils-namespace branch 4 times, most recently from 1fe19c6 to 2a6cec2 Compare August 26, 2023 16:50

mtsokol commented Aug 28, 2023

Copy link
Copy Markdown
Member Author

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 commented Aug 28, 2023

Copy link
Copy Markdown
Member

Two questions:

mtsokol mentioned this pull request Aug 28, 2023
Merged

mtsokol commented Aug 28, 2023

Copy link
Copy Markdown
Member Author 
* 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 self-assigned this Aug 29, 2023
mtsokol force-pushed the lib-arrayutils-namespace branch from 64bfe77 to 3cb96b6 Compare August 30, 2023 09:52

ngoldbaum commented Aug 30, 2023

Copy link
Copy Markdown
Member

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

mtsokol force-pushed the lib-arrayutils-namespace branch from 3cb96b6 to 5fe1e1c Compare September 1, 2023 10:49
mtsokol force-pushed the lib-arrayutils-namespace branch from 5fe1e1c to 4dd54b5 Compare September 6, 2023 08:31
BvB93 reviewed Sep 6, 2023
View reviewed changes
Comment thread numpy/lib/array_utils.pyi
seberg reviewed Sep 6, 2023
View reviewed changes
Comment thread numpy/lib/_array_utils_impl.py
rgommers approved these changes Sep 9, 2023
View reviewed changes

rgommers left a comment

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 commented Sep 11, 2023

Copy link
Copy Markdown
Member

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

In it goes. Thanks @mtsokol.

ngoldbaum merged commit 4f84d71 into numpy:main Sep 11, 2023
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
TYP: Explicitly re-export np.lib.array_utils
d89f176 
xref numpy#24540
brendan-m-murphy mentioned this pull request Nov 11, 2024
Closed
11 tasks

ntessore commented Nov 21, 2024

Copy link
Copy Markdown
Contributor

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 commented Nov 21, 2024

Copy link
Copy Markdown
Member

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

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

seberg seberg left review comments
BvB93 BvB93 left review comments
rgommers rgommers approved these changes

Assignees

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

Back | FazBrowse Home | New Git URL