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

Support Python 3.13 #3805

Open
wants to merge 50 commits into
base: main
Choose a base branch
from
Open

Support Python 3.13 #3805

wants to merge 50 commits into from

Conversation

valeriupredoi
Copy link
Contributor

@valeriupredoi valeriupredoi commented Nov 4, 2024

Description

Sister PR to ESMValGroup/ESMValCore#2566

so far the major bottleneck is ncl which has an autoPR for upgrade to newer proj conda-forge/ncl-feedstock#156 but that fails atm due to inconsistencies in conda-forge

Full list of incompatible dependencies (28/11/2024)

Curiously, the env fails to solve for Python <3.13, even though it solves reasonably fast for Python==3.13 -> NCL's fault


Before you get started

Checklist

It is the responsibility of the author to make sure the pull request is ready to review. The icons indicate whether the item will be subject to the 🛠 Technical or 🧪 Scientific review.

@valeriupredoi valeriupredoi added the iris311 needs iris 3.11 stable release label Nov 5, 2024
@valeriupredoi
Copy link
Contributor Author

valeriupredoi commented Nov 25, 2024

OK I removed ncl for now (clearly a blocker, noqa), we need to rebuild pys2index for py3.13; next hurdle psy- packages! EDIT: just psy-simple actually

see main PR description

environment.yml Outdated
- pyproj >=2.1
- pys2index # only from conda-forge
- python >=3.10,<3.13
# - pys2index # only from conda-forge # not py313 compat
Copy link
Member

Choose a reason for hiding this comment

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

It looks like this is solved, so now we're stuck on the psy-* packages and an unmaintained Julia installation, is that right?

Copy link
Member

Choose a reason for hiding this comment

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

Or is NCL also an issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes and yes, and no - technically NCL is not a problem, but it brings down a bunch of deps to old versions (like esmpy/mf) so that may cause us headaches very soon, let me revive this PR now, bud 🍺

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well poop - now the env fails to solve again, at least this time it offers a clean fail for 3.13 - back to the drawing board see what's causing it 🤦‍♂️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll be a monkey's uncle 🐵 Mamba is indeed correct:

---
name: testenv13
channels:
  - conda-forge
  - nodefaults

dependencies:
  - python=3.13
  - cartopy
  - esmvalcore 2.12*
  - iris >=3.11
  - shapely >=2.0.2

indeed fails to solve - what a PITA - I checked each of these deps and they should be fine individually wrt Python 3.13, time to get some butts kicked then 🛴

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the red bit (yes, locally, one gets stuff in green and red, which is handy) is this:

    │  └─ cartopy 0.24.0 would require
    │     ├─ python >=3.13.0rc3,<3.14.0a0 *, which conflicts with any installable versions previously reported;

I am absolutely baffled, since cartopy 0.24 is Py313ed since September conda-forge/cartopy-feedstock#160

Copy link
Contributor Author

Choose a reason for hiding this comment

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

got it! It's something in esmvalcore that's preventing that mini-env to solve

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well this is the one - python >={{ python_min }},<3.13 - how the heck did this go through? Passed my review no less, but I suspect this is from a subsequent auto conda build 😠

@valeriupredoi
Copy link
Contributor Author

@bouweandela in Borat's words - great success! Py313 env solves, and, bar Julia stuff (ours), and psy- stuff (theirs), all seems to be working well https://github.com/ESMValGroup/ESMValTool/actions/runs/14085944352/job/39515450772?pr=3805
Note, however, the insanely long times it takes the env to solve (even for 3.13 at 13min, but a lot worse for lower Pythons) - that's all due to NCL, look at OSX solving in mere minutes. We need to be very careful about what we do next wrt our environment, currently that pin on curl (needed by Julia) is actually holding this together, but deps are starting to get old 🍺

@bouweandela
Copy link
Member

Removing the upper pin on R makes the environment solve in about 80 seconds on my laptop, maybe it is time we fix those R recipes..

@valeriupredoi
Copy link
Contributor Author

Removing the upper pin on R makes the environment solve in about 80 seconds on my laptop, maybe it is time we fix those R recipes..

was just about to go there (the r-base pin) right now, after seeing that NCL is not really the main culprit

@valeriupredoi
Copy link
Contributor Author

the rbase pin does the trick for 3.13, lower Pythons still struggle - think this one really is NCL - lemme check https://github.com/ESMValGroup/ESMValTool/actions/runs/14109536639/job/39524308496?pr=3805

@valeriupredoi
Copy link
Contributor Author

yup, it is NCL

@valeriupredoi
Copy link
Contributor Author

valeriupredoi commented Mar 27, 2025

OK @bouweandela this is how it looks so far:

  • indeed the rbase pin needs to go: this makes 313 solve fast, but lower pythons don't
  • NCL needs to go otherwise lower Pythons take forever
  • if we remove importlib_metadata and its pin, none solve in geophysical timeframes (even though that importlib_metadata is there only since NCL forces old esmpy/esmf) NCL then Python 3.13 takes forever (!!)

What say you? I am this close to saying we ought to keep just a pure Python environment, and let anyone else who needs R, NCL, nco etc get their own deps and install them 😁

@valeriupredoi
Copy link
Contributor Author

also, the way we are trying to get all these elephants through a key hole is absolutely not sustainable, and in the future, we may have to do the same for, say, maybe, Python 3.14 etc

@valeriupredoi
Copy link
Contributor Author

Look at the jobs run with esmvaltool_python.yml instead of the bloated environment.yml - https://github.com/ESMValGroup/ESMValTool/actions/runs/14111301267/job/39530430869?pr=3805 - the comparison is humbling

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
installation iris311 needs iris 3.11 stable release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants