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

also migrate python 3.12 for numpy 2.0; "soft-close" 3.12 migration #5851

Merged
merged 3 commits into from
May 15, 2024

Conversation

h-vetinari
Copy link
Member

Otherwise we'll need another migration just for 3.12, which is even more of a hassle than dealing with superfluous migrator PRs now. For more details/discussion, see conda-forge/conda-forge.github.io#1997

@h-vetinari h-vetinari requested a review from a team as a code owner May 6, 2024 00:57
@conda-forge-webservices
Copy link
Contributor

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@xhochy
Copy link
Member

xhochy commented May 6, 2024

To understand correctly: This is now a combined migration that would pull a feedstock onto NumPy 2 and add Python 3.12 builds?

OK with me, just want to make sure I understand it.

@beckermr
Copy link
Member

beckermr commented May 6, 2024

Agreed this is fine but also maybe we just close the python 3.12 migration? That will have the same effect for PRs in the numpy2 migration and it is May anyways so why not?

Copy link
Contributor

@hmaarrfk hmaarrfk left a comment

Choose a reason for hiding this comment

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

I would like to wait until a few numpy core maintainers have finished their disucssion as to whether or not python 3.9 should be even be included.

https://mail.python.org/archives/list/numpy-discussion@python.org/thread/AHTATJKGUEOILBNUI5IGGZPXJ5FXIRAU/

@h-vetinari
Copy link
Member Author

@beckermr: Agreed this is fine but also maybe we just close the python 3.12 migration? That will have the same effect for PRs in the numpy2 migration and it is May anyways so why not?

We still have around ~900 unmigrated feedstocks for 3.12 (see comment). I think this is probably too much...? Also, in recent years we've closed the 3.X migration once we started migrating for 3.{X+1}.

@xhochy: To understand correctly: This is now a combined migration that would pull a feedstock onto NumPy 2 and add Python 3.12 builds?

Yes exactly, and it overrides existing python312.yaml files as well (because "2.0" > "1.26").

The downside is that we'd be migrating all python-feedstocks (not just all numpy ones), see same comment.

@xhochy
Copy link
Member

xhochy commented May 7, 2024

Also, in recent years we've closed the 3.X migration once we started migrating for 3.{X+1}.

That is also only a coincidence of someone spending time on the Python migrations in general. After 4-6 months most packages have been migrated to the new Python version or will never be migrated as they are no longer maintained. @ocefpaf or myself have gone through all open PRs in the last years and were able to migrate a lot of feedstocks were upstream was actively maintained and the feedstocks not. I think once someone has done some passes like that, we can close 3.12.

@h-vetinari
Copy link
Member Author

I think once someone has done some passes like that, we can close 3.12.

Well, in that sense, going with the PR as-is would give every feedback another shot at being migrated for python 3.12, and with the attention the numpy2 migration will get, we could then definitely close it.

But I'm also fine with closing the 312 migration beforehand to avoid the PR churn, if people prefer that overall.

Copy link
Contributor

@hmaarrfk hmaarrfk left a comment

Choose a reason for hiding this comment

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

My concerns are no longer. thank you for waiting.

@jakirkham
Copy link
Member

Sorry if I'm missing something, but have we already tested these changes in a feedstock somewhere?

@h-vetinari
Copy link
Member Author

Thanks @hmaarrfk.

@conda-forge/core - I'd like to have some explicit (dis)agreement here... Do we:

  1. think we can handle the extra PR churn (it would be easy just to close PRs if the feedstock doesn't build against numpy)?
  2. prefer to just close the python 3.12 migration and put the onus on feedstocks that haven't migrated py312 yet to skip it for the time being?

I think 2. is more friendly to feedstocks that are "good citizens" and kept up-to-date, whereas 1. gives more time for people to finish migrating for 3.12, in exchange for more migration PR churn.

@h-vetinari
Copy link
Member Author

Sorry if I'm missing something, but have we already tested these changes in a feedstock somewhere?

Yes I did, but didn't push my changes, because the (already 2.0-migrated) feedstock didn't change - which is as it should be.

@jakirkham
Copy link
Member

So I tried out the NumPy 2 migrator on a feedstock that uses NumPy via Python only (no C API). Here is the PR: conda-forge/numcodecs-feedstock#106

AFAICT the only change it makes is adding conda-forge/label/numpy_rc to channel_sources. Am guessing this is what we expect?

If so, then the only reason the migrator will open a migration PR to a feedstock not using NumPy (in build or at all) is this channel addition. So if we were able to handle this channel addition another way, the migration bot would actually skip feedstocks (like this one) where NumPy is used at runtime or not at all

So how might we do this? Some options:

  1. Add the channel in conda-forge-ci-setup
  2. Add the channel globally and constrain all numpy versions to <2 (which the migrator then removes)
  3. Add the channel in the minimigrator
  4. Having the NumPy 2 migrator wait for the NumPy 2 minimigrator (we did this successfully with the CUDA 11.2 arch migrator)
  5. ...?

Please feel free to suggest more

Admittedly all of these have their tradeoffs. That said, maybe this is a way we could find the balance between having the Python 3.12 migrator stay open a bit while keeping the NumPy 2 migrator focused on relevant feedstocks

@h-vetinari
Copy link
Member Author

Thanks for the testing and additional ideas!

Some options:

If I understand you correctly, then 1. & 2. need conda-forge/conda-forge-repodata-patches-feedstock#712, which has it's own set of problems (i.e. patching ~100k artefacts; though I'd be in favour of it...).

Not sure about 3. - the thing needs to be rerender-proof. How would you add it in the piggyback so that it persists (except literally writing channel_sources: to the local CBC, which sounds too intrusive to me, and also hard to undo later).

I think 4. is not feasible currently. There's a piggyback that attaches to some migration (or even all of them, if you really want), but it's not a migrator in and of itself that can be awaited (unlike the alt-arch migration).

If so, then the only reason the migrator will open a migration PR to a feedstock not using NumPy (in build or at all) is this channel addition.

To be fair, any feedstock that hasn't been rerendered in a few weeks will produce a non-empty diff upon rerendering, and so the bot will open a PR regardless, because it cannot (or at least does not) distinguish based on the kind of changes that the diff produces (c.f. regro/cf-scripts#2500).

@h-vetinari
Copy link
Member Author

@conda-forge/core - I'd like to have some explicit (dis)agreement here...

While waiting for responses here, I've started looking at winding down the python 3.12 migration: #5892

@h-vetinari h-vetinari changed the title also migrate python 3.12 for numpy 2.0 also migrate python 3.12 for numpy 2.0; "soft-close" 3.12 migration May 15, 2024
@h-vetinari
Copy link
Member Author

h-vetinari commented May 15, 2024

For now I've followed the good idea of @hmaarrfk in #5892 to "soft-close" the 3.12 migration, as in: apply the changes of the migrator to the global pinning already, but do not delete the migrator file.

Compared to outright closing the migration, this has the advantage that we can still follow the bot's progress on the status page and the bot will also keep raising PRs for feedstocks once all their dependencies become available (even though, strictly speaking, a rerender of said feedstock would already pull in 3.12; feedstocks with missing dependencies would then have to add skip: true # [py>311] until the bot come by).

I've tested the state of this PR in conda-forge/pythran-feedstock#85 (the global pinning cannot be faked by use_local: true, but it's possible to override it when using conda smithy rerender -e <global_pinning>), and it works fine.

@h-vetinari
Copy link
Member Author

This was just discussed in the core call and received a lot of support and no opposition.

@h-vetinari h-vetinari merged commit 64c1048 into conda-forge:main May 15, 2024
4 checks passed
@h-vetinari h-vetinari deleted the numpy branch May 15, 2024 18:11
@jakirkham
Copy link
Member

Thanks Axel! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants