Skip to content

API: Update lib.shape_base namespace #24566

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

Merged
merged 3 commits into from
Aug 31, 2023

Conversation

mtsokol
Copy link
Member

@mtsokol mtsokol commented Aug 28, 2023

Relevant issue #24507

Hi @rgommers @ngoldbaum,

This PR moves lib.shape_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.

Additionally this PR removes np.get_array_wrap from the main namespace as it was already deprecated this month and it isn't used in SciPy, matplotlib, pandas etc. It also deprecates np.lib.shape_base.get_array_prepare (copy-paste of get_array_wrap implementation) that isn't even exposed in main namespace or used it other libraries.

@mtsokol mtsokol self-assigned this Aug 28, 2023
@mtsokol mtsokol force-pushed the lib-shapebase-namespace branch 2 times, most recently from f51caf1 to eb29adc Compare August 28, 2023 15:58
@ngoldbaum
Copy link
Member

Despite get_array_prepare not being deprecated yet, I think it's probably OK to just go ahead and remove it, since it has even less usage than get_array_wrap and it never appeared in the main namespace.

@rgommers
Copy link
Member

I think it's probably OK to just go ahead and remove it, since it has even less usage than get_array_wrap and it never appeared in the main namespace.

Agreed. It was never public to begin with, and I've never seen it used anywhere.

@mtsokol mtsokol force-pushed the lib-shapebase-namespace branch from eb29adc to c84e087 Compare August 30, 2023 09:30
DeprecationWarning,
stacklevel=2
)

Copy link
Member

Choose a reason for hiding this comment

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

IMO no need to deprecate it, just remove it. It has no internal consumers and as far as I can see no public consumers on github besides wrappers for the entire numpy API.

Copy link
Member Author

@mtsokol mtsokol Aug 30, 2023

Choose a reason for hiding this comment

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

I agree it's even better option for that one - removed.

@mtsokol mtsokol force-pushed the lib-shapebase-namespace branch from c84e087 to 68ffec2 Compare August 30, 2023 18:10
@mtsokol mtsokol force-pushed the lib-shapebase-namespace branch from 68ffec2 to e03d8ff Compare August 30, 2023 18:11
@mtsokol mtsokol requested a review from ngoldbaum August 31, 2023 07:42
@ngoldbaum
Copy link
Member

The only change to the main namespace is get_array_wrap:

In [7]: set(oldall) - set(np.__all__)
Out[7]: {'get_array_wrap'}

It also looks like only items available in the main namespace were removed from np.lib.__all__ (except get_array_wrap):

In [12]: set(oldliball) - set(np.lib.__all__)
Out[12]: 
{'apply_along_axis',
 'apply_over_axes',
 'array_split',
 'column_stack',
 'dsplit',
 'dstack',
 'expand_dims',
 'get_array_wrap',
 'hsplit',
 'kron',
 'put_along_axis',
 'row_stack',
 'split',
 'take_along_axis',
 'tile',
 'vsplit'}

In [13]: diff = set(oldliball) - set(np.lib.__all__)

In [14]: diff.issubset(np.__all__)
Out[14]: False

In [15]: diff - set(np.__all__)
Out[15]: {'get_array_wrap'}

So I think that all the API changes are correct.

@ngoldbaum
Copy link
Member

Thanks @mtsokol!

@ngoldbaum ngoldbaum merged commit 20d03bc into numpy:main Aug 31, 2023
@mtsokol mtsokol deleted the lib-shapebase-namespace branch August 31, 2023 19:01
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.

3 participants