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

API: Update arraypad,arraysetops, ufunclike and utils namespaces in np.lib #24567

Merged
merged 4 commits into from Aug 29, 2023

Conversation

mtsokol
Copy link
Member

@mtsokol mtsokol commented Aug 28, 2023

Relevant issue #24507

Hi @rgommers @ngoldbaum,

This PR moves lib.arraypad, lib.arraysetops, lib.ufunclike and lib.utils modules to private files and ensures that their public methods are only available through the main namespace. There are no additional functions available from their local namespaces.

I decided to join those four submodules together as they have small number of functions (e.g. arraypad has only one).

@mtsokol mtsokol changed the title API: Update arraypad,arraysetops, ufunclike and utils namespaces in lib` API: Update arraypad,arraysetops, ufunclike and utils namespaces in lib Aug 28, 2023
@mtsokol mtsokol changed the title API: Update arraypad,arraysetops, ufunclike and utils namespaces in lib API: Update arraypad,arraysetops, ufunclike and utils namespaces in np.lib Aug 28, 2023
@mtsokol mtsokol self-assigned this Aug 28, 2023
@mtsokol mtsokol force-pushed the lib-pad-setops-ufunc-utils-namespace branch from 6b6f09c to 8ce7e9e Compare August 28, 2023 15:56
@ngoldbaum
Copy link
Member

len(np.lib.__all__) drops from 87 to 71 with this PR. Everything removed is still available in the top-level namespace.

Copy link
Member

@ngoldbaum ngoldbaum left a comment

Choose a reason for hiding this comment

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

I'm seeing some downstream use of patterns like this:

unique_src_ids = np.lib.arraysetops.unique(src_ids) 

It would probably be worthwhile to add errors redirecting people to the main namespace or whichever other migration path is appropriate for that submodule if they try to access any np.lib submodules we're removing.

doc/source/reference/routines.set.rst Outdated Show resolved Hide resolved
@ngoldbaum
Copy link
Member

It would probably be worthwhile to add errors redirecting people to the main namespace or whichever other migration path is appropriate for that submodule if they try to access any np.lib submodules we're removing.

This could be a followup though, no need to hold up merging this, which looks good otherwise except for the one issue I pointed out in the docs.

@mtsokol
Copy link
Member Author

mtsokol commented Aug 28, 2023

It would probably be worthwhile to add errors redirecting people to the main namespace or whichever other migration path is appropriate for that submodule if they try to access any np.lib submodules we're removing.

This could be a followup though, no need to hold up merging this, which looks good otherwise except for the one issue I pointed out in the docs.

@ngoldbaum I think that's a good point - I added it here to def __getattr__(): in numpy/lib/__init__.py. If a user wants to access a function from a lib module like:

np.lib.arraysetops.unique(src_ids) 

The message says:

AttributeError: `np.lib.arraysetops` is now private. Function that you're looking for should be available from the main namespace `np.*`

@mtsokol mtsokol force-pushed the lib-pad-setops-ufunc-utils-namespace branch 2 times, most recently from dbf38c5 to 35c133a Compare August 28, 2023 19:36
numpy/lib/__init__.py Outdated Show resolved Hide resolved
@rgommers rgommers added this to the 2.0.0 release milestone Aug 29, 2023
@mtsokol mtsokol force-pushed the lib-pad-setops-ufunc-utils-namespace branch from f29c280 to 08e086c Compare August 29, 2023 19:13
@ngoldbaum
Copy link
Member

Let's merge this one as well so the other np.lib PRs can get updated to add to the __getattr__ error. Thanks for dealing with all the merge conflicts and making the PR diffs a lot smaller!

@ngoldbaum ngoldbaum merged commit 39bc1b1 into numpy:main Aug 29, 2023
48 of 50 checks passed
@mtsokol mtsokol deleted the lib-pad-setops-ufunc-utils-namespace branch August 29, 2023 20:16
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

4 participants