ESMValGroup / ESMValCore

ESMValCore: A community tool for pre-processing data from Earth system models in CMIP and running analysis scripts.
https://www.esmvaltool.org
Apache License 2.0
42 stars 38 forks source link

[Numpy2] Support for `numpy==2.0.0` #2395

Open valeriupredoi opened 4 months ago

valeriupredoi commented 4 months ago

Description

This PR introduces the changes we need to support Numpy 2.0 (these changes were collected from running the tests, so they may not be 100% exhaustive, but given that we have 95% test coverage, I'd say they are 95% exhaustive; we can deal with missed cases as they come along, if any; I would not use the ruff plugin test Numpy recommends to migrate, since that will introduce a lot of style changes that we may want to make when we include ruff as our linter). Most changes are in our testing suite, but there are a couple in the source code; these are backwards compatible changes, ie all tests pass when running with numpy=1.26.4

The only test that fails (both for Numpy2 and numpy legacy) is related to the pins on pandas, that need to go since we need to use the latest pandas for environment compatibility, see https://github.com/ESMValGroup/ESMValCore/issues/2459 ; I'll try fix that issue here, since it's related to (but not a direct consequence of) Numpy2.

Closes #2460 Closes https://github.com/ESMValGroup/ESMValCore/issues/2459

The second issue closue is more of a brush it under the carpet via a pytest xfail, the original issue that kept creeping up is https://github.com/ESMValGroup/ESMValCore/pull/2305 and the actual issue is with Pandas, and listed in https://github.com/pandas-dev/pandas/issues/57002 - we can wait on its fix (and have us test quietness via the test xfail) or @schlunma can propose a more elegant solution in a separate PR?

Historical guff

OK @ESMValGroup/technical-lead-development-team we finally have a correct set of tests with the latest Numpy=2.0.0 (stable) - don't mind the ones related to pandas, I had to unpin it to gte pandas=2.2.2 built with Numpy 2.0 (otherwise the environment wouldn't solve) -> we have some work to do :+1: https://github.com/ESMValGroup/ESMValCore/actions/runs/9582595825/job/26421917358?pr=2395

~Numpy 2.0 is impending~: it is now fully out (see https://anaconda.org/conda-forge/numpy ); resources https://anaconda.org/conda-forge/numpy/files and https://github.com/numpy/numpy/releases - they recommend a test with the RC and list the changes roadmap in https://numpy.org/devdocs/numpy_2_0_migration_guide.html so we can test ourselves. Note: this branch should not be merged! If everything works out fine, we'll just pick up numpy 2.0 as per normal. ATTN: @ESMValGroup/technical-lead-development-team

codecov[bot] commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 94.70%. Comparing base (ecb01b3) to head (0b2e975).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2395 +/- ## ======================================= Coverage 94.70% 94.70% ======================================= Files 249 249 Lines 14114 14118 +4 ======================================= + Hits 13367 13371 +4 Misses 747 747 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

valeriupredoi commented 4 months ago

UGH! This doesn't look good at all https://github.com/ESMValGroup/ESMValCore/actions/runs/8664961338/job/23762564320

bouweandela commented 4 months ago

xref https://github.com/Unidata/cftime/issues/318

valeriupredoi commented 4 months ago

Spot on @bouweandela 🍻 Fortunately there is a mature PR for it https://github.com/Unidata/cftime/pull/319

valeriupredoi commented 4 months ago

K then, just waiting for a new cftime to be released

valeriupredoi commented 3 months ago

still royally hosed even with rc2

valeriupredoi commented 2 months ago

still need an updated ABI, it's a bit too early now; we'll have to rerun

valeriupredoi commented 2 months ago

OK environment barfs due to python-stratify, that has an old autobot PR to update to numpy 2.0 https://github.com/conda-forge/python-stratify-feedstock/pull/26 - will go ping there EDIT: Filipe has taken care of that :partying_face:

valeriupredoi commented 2 months ago

OK @ESMValGroup/technical-lead-development-team we finally have a correct set of tests with the latest Numpy=2.0.0 (stable) - don't mind the ones related to pandas, I had to unpin it to gte pandas=2.2.2 built with Numpy 2.0 (otherwise the environment wouldn't solve) -> we have some work to do :+1: https://github.com/ESMValGroup/ESMValCore/actions/runs/9582595825/job/26421917358?pr=2395

bouweandela commented 1 month ago

Maybe we should wait for Iris to be numpy 2 compatible: https://github.com/SciTools/iris/pull/6035

bouweandela commented 1 month ago

It looks like that will happen when Iris 3.11 is released at the earliest, planned in October: https://github.com/SciTools/iris/discussions/5571#discussioncomment-9792428.

valeriupredoi commented 1 month ago

many thanks for the reviews and comments @schlunma and @bouweandela :beer: x2 I don't think we can wait until October TBF, I say we get this in and see what's the worst it can happen, and fairly sure not much, and if there are problems, we can alert iris and tell them to hurry up :grin:

bouweandela commented 1 month ago

I say we get this in and see what's the worst it can happen

I'm sorry, but I have to disagree. What would most likely happen is that users will get wrong results and/or crashes.

valeriupredoi commented 1 month ago

I say we get this in and see what's the worst it can happen

I'm sorry, but I have to disagree. What would most likely happen is that users will get wrong results and/or crashes.

that is indeed the possibility; I guess there's one way to find out about the magnitude of that - run the iris tests with a numpy>=2 pin in an iris fork