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: Remove several niche objects for numpy 2.0 python API cleanup #24144

Merged
merged 6 commits into from Jul 27, 2023

Conversation

ev-br
Copy link
Contributor

@ev-br ev-br commented Jul 8, 2023

As discussed in #23999 (comment), specifically #23999 (comment), remove

  • np.cast
  • np.info
  • np.source
  • np.lookfor

An alternative is of course to formally deprecate them first :-).

@ngoldbaum
Copy link
Member

I think np.info was supposed to stick around.

@WarrenWeckesser
Copy link
Member

There has been recent activity related to info, e.g. #23245, #23355.

I'm ambivalent about the ultimate fate of info, but if it is to be removed, the removal must be consistent with the usual deprecation policy. Let's not get too trigger happy with removing stuff without the correct deprecation cycle; we should not fall into the normalization of deviance where we get casual about removing seemingly insignifcant things without deprecation. There should be a really compelling reason for not sticking to the standard deprecation process.

@ev-br
Copy link
Contributor Author

ev-br commented Jul 8, 2023

OK, updated with np.info staying intact.

@rgommers
Copy link
Member

rgommers commented Jul 9, 2023

. Let's not get too trigger happy with removing stuff without the correct deprecation cycle; we should not fall into the normalization of deviance where we get casual about removing seemingly insignifcant things without deprecation. There should be a really compelling reason for not sticking to the standard deprecation process.

There is a compelling reason, namely NumPy 2.0 which already requires new releases and wheel builds from every significant downstream package. I think this is the wrong PR to make this comment; this PR seems fine to me given
https://numpy.org/neps/nep-0052-python-api-cleanup.html#backward-compatibility. Having it deprecated in 2.0 and then removed in 2.2 is kind of pointless work for these APIs, and has a negative impact on one of the key goals for 2.0.

@rgommers
Copy link
Member

rgommers commented Jul 9, 2023

I had a look at np.info and the linked issues - it seems like it is useful to some users, but the docstring for it is completely wrong - it is not at all equivalent to help/?.

@seberg
Copy link
Member

seberg commented Jul 9, 2023

I would like to always think of the two things:

  • who is affected: in this case at least probably very few, and most of the functions are docs, so no-one in production
  • how are they affected: I think in this case the replacement is rather obvious.

I will grant that np.info has probably uses and users at least for teaching, so happy to keep it.

@seberg
Copy link
Member

seberg commented Jul 10, 2023

@ev-br can you add brief release notes? Maybe with a sentence saying to replace np.cast with the call to .astype() or np.asarray(..., dtype=)? (I guess for source, ?? or inspect may also be something we could point to)

EDIT: Forgot to metion, there is a python_removal category (iirc, there should be one fragment/file already)

@seberg
Copy link
Member

seberg commented Jul 10, 2023

But, if anyone disagrees, I don't mind making it a deprecation, I just think the step adds little beyond allowing downstream to work without updates, and large chunks of downstream won't have that (and this won't be the reason why).

@ev-br
Copy link
Contributor Author

ev-br commented Jul 10, 2023

can you add brief release notes

Sure, done.

@seberg seberg changed the title Remove several niche objects for numpy 2.0 python API cleanup API: Remove several niche objects for numpy 2.0 python API cleanup Jul 10, 2023
@seberg
Copy link
Member

seberg commented Jul 10, 2023

Test failures are unfortunately real, there seem to be remains of these in the typing tests.

@ev-br
Copy link
Contributor Author

ev-br commented Jul 13, 2023

CI is 🟢

@seberg
Copy link
Member

seberg commented Jul 13, 2023

Grrrrrrr, I did a github code search found a decent amount of np.cast[ usage. Considering that it's ugly but trivial code, probably it's better to only deprecate it :(.

I can follow up with that and finish that, though.

@rgommers
Copy link
Member

Grrrrrrr, I did a github code search found a decent amount of np.cast[ usage. Considering that it's ugly but trivial code, probably it's better to only deprecate it :(.

If it's trivial to update, I think we should remove it. And otherwise we have to ensure it's not in the reference guide anymore - in this particular case that's already true though, so no extra work (but also an extra reason to remove, it was never even meant as public API).

@seberg
Copy link
Member

seberg commented Jul 27, 2023

Ok, let's roll with it. I will note that if anyone complains about np.cast with some conviction, I will vote to revert it. I don't really think its worth (+pushing through/spending many "annoy users" chips on).

The replacement suggesting might not create a copy when there previously was, but I don't think that should be a problem, but rather an improvement in most cases.

@seberg seberg merged commit e14d9c8 into numpy:main Jul 27, 2023
57 checks passed
@seberg seberg added this to the 2.0.0 release milestone Jul 27, 2023
@jakevdp
Copy link
Contributor

jakevdp commented Jul 27, 2023

One note: removing cast breaks scipy, which imports it here: https://github.com/scipy/scipy/blob/v1.11.1/scipy/linalg/_decomp.py#L22

Discovered because JAX's upstream nightly CI failed: google/jax#16864

Should there be some kind of deprecation cycle here? Left as is, the next numpy release will cause all existing scipy releases to error on import.

@ev-br
Copy link
Contributor Author

ev-br commented Jul 27, 2023

scipy usage will get fixed later today, thanks for finding it!
I believe released scipy versions have an upper pin for numpy < 2.0.

@seberg
Copy link
Member

seberg commented Jul 27, 2023

I am not worried about SciPy since it must adapt to 2.0 anyway as a C-API user. But it is a data-point towards it being used enough to go through a deprecation to help smooth transition for small less maintained Python libs that may also use it.

@ev-br
Copy link
Contributor Author

ev-br commented Jul 27, 2023

scipy PR: scipy/scipy#18973

@rgommers
Copy link
Member

rgommers commented Jul 27, 2023

I will write up a short proposal here and send it to the mailing list. Basically everything we want to remove is used somewhere, and what I would like to avoid is that we avoid any pain in 2.0 and as a result make 2.2 the major breaking release. That'd be worse. There may be things that indeed do deserve only a deprecation and removal in 2.2 or even later, but we cannot do that for every niche API. EDIT: done in this message

F3eQnxN3RriK added a commit to F3eQnxN3RriK/numpy that referenced this pull request Jul 28, 2023
``np.source`` and ``np.lookfor`` was removed (numpygh-24144).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

6 participants