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
40 stars 36 forks source link

Updated esmf-related pins #2445

Closed schlunma closed 1 month ago

schlunma commented 1 month ago

Description

With the new release of iris-esmf-regrid we can update some pins in the environment.

See also https://github.com/SciTools-incubator/iris-esmf-regrid/pull/342#issuecomment-2092921514.


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.


To help with the number pull requests:

codecov[bot] commented 1 month ago

Codecov Report

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

Project coverage is 94.52%. Comparing base (57d1d05) to head (86144a5).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2445 +/- ## ======================================= Coverage 94.52% 94.52% ======================================= Files 246 246 Lines 14036 14036 ======================================= Hits 13267 13267 Misses 769 769 ```

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

valeriupredoi commented 1 month ago

@schlunma I'd argue this would be nice to have in v2.11 if @chrisbillowsMO is not already too fed up with accepting any more bugfixes :beer:

valeriupredoi commented 1 month ago

OK we have a problem with the environment for Python 3.9 - they require Python >=3.10 so our envs don't solve https://github.com/ESMValGroup/ESMValCore/actions/runs/9350804150/job/25735049553 - we either retire support for Python 3.9 as per @bouweandela 's https://github.com/ESMValGroup/ESMValCore/issues/2406 now or we don't include this in the release and drop 3.9 the sooner the better - I am leaning towards not dropping 3.9 before the release (and hence, not including this in the release)

ehogan commented 3 weeks ago

OK we have a problem with the environment for Python 3.9 - they require Python >=3.10 so our envs don't solve https://github.com/ESMValGroup/ESMValCore/actions/runs/9350804150/job/25735049553 - we either retire support for Python 3.9 as per @bouweandela 's #2406 now or we don't include this in the release and drop 3.9 the sooner the better - I am leaning towards not dropping 3.9 before the release (and hence, not including this in the release)

Unfortunately, esmf-related pins were updated in ESMValTool via https://github.com/ESMValGroup/ESMValTool/pull/3643 already, which means it's not possible to solve an environment using the latest version of ESMValTool (on main) and the ESMValCore release branch, since ESMValTool has esmpy >=8.6.0 and the ESMValCore release branch has esmpy !=8.1.0,<8.6.0 ๐Ÿ˜ฎ

So either we should just include this in the release, or I will need to create an ESMValTool release branch from the commit before https://github.com/ESMValGroup/ESMValTool/pull/3643 (from 3 weeks ago!) and cherry pick everything after that into the release branch ๐Ÿ˜ญ

What would be best? ๐Ÿค”

valeriupredoi commented 3 weeks ago

@ehogan that pin needs to be in the ESMValCore release to allow for uniformity with ESMValTool; having said that, that exact pin creates incompatibility between current ESMValTool and Python=3.12 via NCL not being able to support ESMPy/esmf>=8.6.0, something that will take a while to resolve at upstream (NCL), so if that pin is not absolutely necessary, I'd be very happy to see it dropped - I care more for Python 3.12 support than some sciencey thingie :laughing:

bouweandela commented 3 weeks ago

How about you create the ESMValTool release branch from main and then revert the changes introduced in https://github.com/ESMValGroup/ESMValTool/pull/3643 only in the ESMValTool release branch?

bouweandela commented 3 weeks ago

That would resemble the conda environment that you did all the tests with best, right?

ehogan commented 3 weeks ago

How about you create the ESMValTool release branch from main and then revert the changes introduced in ESMValGroup/ESMValTool#3643 only in the ESMValTool release branch?

Ah, yes, this sounds good, I will try this ๐Ÿ‘

valeriupredoi commented 3 weeks ago

How about you create the ESMValTool release branch from main and then revert the changes introduced in ESMValGroup/ESMValTool#3643 only in the ESMValTool release branch?

and make sure you don't include the pin in the conda package via recipe.meta :+1:

schlunma commented 3 weeks ago

If the change in iris-esmf-regrid is fully backwards-compatible, we could just relax the pin to esmpy!=8.1.0, but we still don't know this yet ๐Ÿคท

valeriupredoi commented 3 weeks ago

If the change in iris-esmf-regrid is fully backwards-compatible, we could just relax the pin to esmpy!=8.1.0, but we still don't know this yet ๐Ÿคท

aha! Good catch, Manu! I asked the same q myself - I'd go along the "what's the worst it can happen" line, and stash the pin for now, hopefully they'll give us an answer, and by then NCL will have had enough time to support the new ESMPy :crossed_fingers: