Skip to content

API: Update lib.function_base namespace #24538

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
merged 3 commits into from
Aug 27, 2023
Merged

API: Update lib.function_base namespace #24538

merged 3 commits into from
Aug 27, 2023

Conversation

mtsokol
Copy link
Member

@mtsokol mtsokol commented Aug 25, 2023

Relevant issue #24507

Hi @rgommers @ngoldbaum,

This PR moves lib.function_base module to a private file and ensures that its public methods are only available through the main namespace. There are no additional functions available from a local namespace.

@mtsokol mtsokol mentioned this pull request Aug 25, 2023
Closed
16 tasks
@mtsokol mtsokol force-pushed the lib-functionbase-namespace branch 3 times, most recently from 877ce20 to 28aef0a Compare August 25, 2023 10:37
@rgommers rgommers added this to the 2.0.0 release milestone Aug 25, 2023
@mtsokol mtsokol force-pushed the lib-functionbase-namespace branch 2 times, most recently from 3a19a4b to 5f20dad Compare August 26, 2023 15:46
@mtsokol mtsokol force-pushed the lib-functionbase-namespace branch from 5f20dad to 5a5d7b8 Compare August 26, 2023 16:16
rgommers
rgommers approved these changes Aug 27, 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, and I verified that this doesn't change anything in np.__all__ ('disp' was already not in it). So in it goes, thanks @mtsokol.

@rgommers rgommers merged commit e865b33 into numpy:main Aug 27, 2023
@mtsokol mtsokol deleted the lib-functionbase-namespace branch August 27, 2023 11:54
@WarrenWeckesser
Copy link
Member

It looks like this removed add_newdoc from numpy.lib. This appears to be the change that is breaking the SciPy builds with the numpy main dev branch (see the CI results for any recent pull request). Back in #12385 (comment), it was stated that add_newdoc would remain public. Has that decision changed? Should we fix the SciPy build by replacing numpy.lib.add_newdoc with our own copy?

@mtsokol
Copy link
Member Author

mtsokol commented Aug 28, 2023

It looks like this removed add_newdoc from numpy.lib. This appears to be the change that is breaking the SciPy builds with the numpy main dev branch (see the CI results for any recent pull request). Back in #12385 (comment), it was stated that add_newdoc would remain public. Has that decision changed? Should we fix the SciPy build by replacing numpy.lib.add_newdoc with our own copy?

@WarrenWeckesser, that's correct add_newdoc and add_docstring needs to stay in numpy.lib - let me revert that one-line change quickly.

@rgommers
Copy link
Member

Ah, this was not an intentional change in this PR. Let's put it back right now and build new nightlies to unbreak SciPy. @mtsokol can you open a new PR for this?

Let's add add_newdoc to the list of functions in the numpy.lib namespace in gh-24507 and make an explicit decision there. In case it gets removed, SciPy and other main downstream libraries should be fixed up first.

@rgommers
Copy link
Member

Synchronous comments, glad they match:)

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

mtsokol commented Aug 28, 2023

I created a PR #24564 that fixes it and readds add_newdoc and add_docstring to np.lib only to align with main namespace and lib namespace design.

@rgommers
Copy link
Member

Fix merged and new nightly wheel builds triggered - they should be available in ~30 minutes or so.

@mtsokol mtsokol self-assigned this Aug 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@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.
3 participants
@mtsokol @WarrenWeckesser @rgommers