NEP: Making details of `np.core` private #11924

eric-wieser · 2018-09-10T01:59:54Z

This is a rough draft of what I had in mind in the comments of #11909.

I don't think I'll have the energy to champion this, and would greatly appreciate if someone else could take over from me

rgommers · 2018-09-10T05:43:47Z

doc/neps/nep-0023-restructuring-np-core.rst

+Backward compatibility
+----------------------
+
+In some cases, there may be members at ``np.core.*.*`` which we intended to be public-facing. Users of these will receive ``DeprecationWarning``s until we lift these members to ``np.core.*``.


I don't think those exist. The whole np.core was never intended to be public (and arguably it isn't, missing underscores or not), so submodules of np.core certainly weren't intended to be public.

As an example, np.core.numeric.normalize_axis_index never made it to the top level.

I'd consider moving that to np.core.normalize_axis_index - I think it's perhaps valuable to have a separation between "apis a user would use" and "apis a library cooperating with numpy would use" - np.* vs np.core.* sounds like a reasonable line to draw

Updated the text with this example

rgommers · 2018-09-10T05:47:25Z

I proposed something similar for scipy in 2011: https://mail.python.org/pipermail/scipy-dev/2011-June/016337.html. Didn't happen in the end.

rgommers · 2018-09-10T05:50:25Z

doc/neps/nep-0023-restructuring-np-core.rst

+
+The proposal here is to:
+
+* Make all of the existing modules within `np.core` private, adding a leading underscore to their names.


Maybe a lot less work: move np.core to np._core (and and import warnings for np.core)

If we want to continue supporting (with deprecation warnings) from numpy.core.numeric import something, this is (marginally) more work, not less - you still need a dummy module for every submodule of core.

@rgommers A similar thought occurred to me. This is beginning to sound like the evolution of the numpy.testing package where we ended up with a couple of stub modules and _private. A clear statement of intent and justification would be good to have.

I see that there is a statement of intent. Maybe we can expand this to a discussion of what we would like NumPy to look like in the long term.

ahaldane · 2018-10-10T16:30:52Z

I think I'm +1 on this.

A clearly demarcated public/private api is a good thing. While maintaining back-compat is very important, so is dev freedom to change things, and sometimes it seems like the pendulum has swung to far to the former. It's not good to unnecessarly trap numpy into back-compat corners, as seems to have happened with these core modules.

The old modules can all be soft-deprecated, so no one's code will break except that they get deprecation warnings.

mattip · 2019-12-05T12:21:38Z

This is a NEP proposal, I think we should either move it forward as a draft NEP for wider disucssion or close the PR.

rgommers · 2019-12-05T17:42:32Z

This is a good idea, so please don't close it.

Move forward: @eric-wieser asked for a volunteer. I think this NEP is probably higher prio than most PRs that are open.

mattip · 2020-07-13T05:49:48Z

This NEP is still not merged to draft state, and still looking for someone to move it forward

charris · 2020-12-18T21:35:09Z

doc/neps/nep-0023-restructuring-np-core.rst

@@ -0,0 +1,165 @@
+==============================================
+NEP 23 — Making details of ``np.core`` private
+==============================================


Number needs updating to 46. Also needs a new file name.

charris · 2020-12-18T21:36:03Z

I'm inclined to put this in for discussion, but needs updating.

charris · 2020-12-18T21:36:18Z

close/reopen.

stefanv · 2021-01-21T22:44:51Z

doc/neps/nep-0023-restructuring-np-core.rst

+	 'shape_base',
+	 'umath']
+
+The publicness of this API surface is contagious - in principle,


Suggested change

The publicness of this API surface is contagious - in principle,

`np.core` and its submodules greatly increase the public API surface of NumPy—in principle, for example,

stefanv · 2021-01-21T22:47:20Z

doc/neps/nep-0023-restructuring-np-core.rst

+
+This is problematic - it means that if ``numeric.py`` contains ``from
+numpy.core.multiarray import array``, then we can never remove that line
+without risking breaking someone using ``np.core.numeric.array``.


Suggested change

without risking breaking someone using ``np.core.numeric.array``.

without risking breaking code importing ``np.core.numeric.array``.

stefanv · 2021-01-21T22:55:11Z

doc/neps/nep-0023-restructuring-np-core.rst

+    relying on implementation details.
+
+
+Implementation


Maybe the implementation code should be left to a PR, that can be linked in. My take was that the Implementation sections are for discussing implementation decisions, but not code.

stefanv · 2021-01-21T22:55:55Z

doc/neps/nep-0023-restructuring-np-core.rst

+somewhat public-facing.  An example of such a function is
+`numpy.core.numeric.normalize_axis_index`, which downstream libraries are
+starting to use to validate axis arguments.  Users of these functions will
+receive ``DeprecationWarning``s until we lift these members to ``np.core.*``.


Not sure what "until we lift these members to np.core" means here. They should probably be lifted to numpy.lib?

rgommers · 2021-01-22T14:50:36Z

I forgot about this NEP. Nice timing to comment on it @stefanv. I'll at least link to it from my upcoming NEP about the array API standard. I'd also be interested to push this forward, just a matter of finding the time .....

rgommers · 2021-01-22T14:52:53Z

Let me also link https://github.com/numpy/numpy/commits/master/numpy/tests/test_public_api.py, which has all the relevant public/private/in-between parts listed.

seberg · 2021-12-15T18:10:41Z

We discussed this today, and were landing a bit on "not really a big urgency". This seems obviously a good idea, I personally do not even feel we need a NEP, unless there is a surprising amount of downstream breakage (but it cannot hurt).

The main concerns are:

Where to put the symbols only available through core, this needs a concrete proposal to decide on.
There may be quite a few PRs that have merge conflict.

Neither seem like big road blocks, and DeprecationWarnings can linger for a while (or even only be added after big downstream packages are fixed).

IMO, the main point here is to do the work, and if we want finish up the NEP in no particular order.

rgommers · 2021-12-15T18:36:17Z

+1 for this

There may be quite a few PRs that have merge conflict.

This was the case for the way we just did things in SciPy (adding back a core.py with a __getattr__ and raising suitable deprecation warnings), but scikit-learn dynamically generates core.py and then there's no merge conflicts at all - Git will understand the git mv action.

Where to put the symbols only available through core, this needs a concrete proposal to decide on.

Are there actually any that should be public? IIRC numpy.core submodules are not in the html docs, so there may not be any objects of interest? There's only very few examples given in the draft here, and they're pretty niche.

seberg · 2021-12-15T18:38:45Z

There's only very few examples given in the draft here, and they're pretty niche.

Yes, I agree they are niche, but I suspect they exist. But yes, even OK to deprecated and see who shouts, probably.

WarrenWeckesser · 2021-12-15T19:11:38Z

Are there actually any that should be public?

At least one, already mentioned in the NEP: normalize_axis_index from numpy.core.multiarray (also in numpy.core.numeric). We use this in SciPy . I asked about this on the mailing list (note that I initially referred to the wrong namespace), and in that thread, @eric-wieser said

When I added this function, it was always my intent for it to be consumed
by downstream packages, but as Sebastian remarks, it wasn't really
desirable to put it in the top-level namespace.

I think I would be reasonably happy to make the guarantee that it would not
be removed (or more likely, moved) without a lengthy deprecation cycle.

The comments in the thread suggest eventually moving the function to a "more public" namespace, e.g. numpy.lib, but not necessarily the top-level namespace.

FWIW, I also noticed the cupy issue cupy/cupy#4522, and I see that dask uses normalize_axis_index and normalize_axis_tuple in the file numpy_compat.py, both imported from numpy.core.numeric.

Update: astropy also uses normalize_axis_index, and normalize_axis_tuple (in a compatibility function that will go away when they no longer support numpy less than 1.18). And numba has a recent pull request to add support for numpy.core.multiarray.normalize_axis_index.

NEP: Making details of np.core private

51b3dc3

eric-wieser added the component: NEP label Sep 10, 2018

eric-wieser mentioned this pull request Sep 10, 2018

MAINT: flake8 cleanups #11909

Merged

eric-wieser force-pushed the nep-namespace branch from b2f5cac to 51b3dc3 Compare September 10, 2018 02:09

rgommers reviewed Sep 10, 2018

View reviewed changes

eric-wieser and others added 2 commits September 9, 2018 23:29

Remove stray comma, give example of np.core.*.* function

5eb64df

formatting, fix hyphen->elongated dash

563f5c4

mattip force-pushed the nep-namespace branch from eb55387 to 563f5c4 Compare September 12, 2018 11:26

eric-wieser mentioned this pull request Oct 10, 2018

MAINT: delay initialization of getlimits (circular imports) #12064

Merged

eric-wieser mentioned this pull request Apr 13, 2019

MAINT: Move umath error helpers into their own module #13322

Merged

eric-wieser added the 25 - WIP label May 11, 2019

seberg added the Priority: high High priority, also add milestones for urgent issues label Jan 17, 2020

eric-wieser mentioned this pull request Jul 14, 2020

API: Review public functionality in arrayprint #16869

Closed

rossbar mentioned this pull request Jul 14, 2020

strip_zeros kwarg no longer available in FloatFormat #16036

Closed

charris changed the title ~~WIP/NEP: Making details of np.core private~~ NEP: Making details of np.core private Dec 18, 2020

charris removed the 25 - WIP label Dec 18, 2020

charris reviewed Dec 18, 2020

View reviewed changes

charris closed this Dec 18, 2020

charris reopened this Dec 18, 2020

stefanv reviewed Jan 21, 2021

View reviewed changes

Base automatically changed from master to main March 4, 2021 02:04

InessaPawson added the triage review Issue/PR to be discussed at the next triage meeting label Dec 13, 2021

InessaPawson removed the triage review Issue/PR to be discussed at the next triage meeting label Jan 12, 2022

charris added the 52 - Inactive Pending author response label Apr 6, 2022

charris closed this Apr 6, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NEP: Making details of `np.core` private #11924

NEP: Making details of `np.core` private #11924

eric-wieser commented Sep 10, 2018

rgommers Sep 10, 2018

eric-wieser Sep 10, 2018

eric-wieser Sep 10, 2018

rgommers commented Sep 10, 2018

rgommers Sep 10, 2018

eric-wieser Sep 10, 2018 •

edited

Loading

charris Sep 10, 2018

charris Sep 10, 2018

ahaldane commented Oct 10, 2018

mattip commented Dec 5, 2019

rgommers commented Dec 5, 2019

mattip commented Jul 13, 2020

charris Dec 18, 2020

rgommers Jan 22, 2021

charris commented Dec 18, 2020

charris commented Dec 18, 2020

stefanv Jan 21, 2021

stefanv Jan 21, 2021

stefanv Jan 21, 2021

stefanv Jan 21, 2021

rgommers commented Jan 22, 2021

rgommers commented Jan 22, 2021

seberg commented Dec 15, 2021

rgommers commented Dec 15, 2021

seberg commented Dec 15, 2021

WarrenWeckesser commented Dec 15, 2021 •

edited

Loading


		The proposal here is to:

		* Make all of the existing modules within `np.core` private, adding a leading underscore to their names.

	The publicness of this API surface is contagious - in principle,
	`np.core` and its submodules greatly increase the public API surface of NumPy—in principle, for example,

	without risking breaking someone using ``np.core.numeric.array``.
	without risking breaking code importing ``np.core.numeric.array``.

NEP: Making details of np.core private #11924

NEP: Making details of np.core private #11924

Conversation

eric-wieser commented Sep 10, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rgommers commented Sep 10, 2018

Choose a reason for hiding this comment

eric-wieser Sep 10, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ahaldane commented Oct 10, 2018

mattip commented Dec 5, 2019

rgommers commented Dec 5, 2019

mattip commented Jul 13, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

charris commented Dec 18, 2020

charris commented Dec 18, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rgommers commented Jan 22, 2021

rgommers commented Jan 22, 2021

seberg commented Dec 15, 2021

rgommers commented Dec 15, 2021

seberg commented Dec 15, 2021

WarrenWeckesser commented Dec 15, 2021 • edited Loading

NEP: Making details of `np.core` private #11924

NEP: Making details of `np.core` private #11924

eric-wieser Sep 10, 2018 •

edited

Loading

WarrenWeckesser commented Dec 15, 2021 •

edited

Loading