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

Updated esmf-related pins #2445

Merged
merged 2 commits into from
Jun 3, 2024
Merged

Updated esmf-related pins #2445

merged 2 commits into from
Jun 3, 2024

Conversation

schlunma
Copy link
Contributor

@schlunma schlunma commented Jun 3, 2024

Description

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

See also SciTools/iris-esmf-regrid#342 (comment).


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:

@schlunma schlunma added the installation Installation problem label Jun 3, 2024
@schlunma schlunma added this to the v2.12.0 milestone Jun 3, 2024
@schlunma schlunma requested a review from valeriupredoi June 3, 2024 12:34
@schlunma schlunma self-assigned this Jun 3, 2024
Copy link

codecov bot commented Jun 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.52%. Comparing base (57d1d05) to head (86144a5).
Report is 36 commits behind head on main.

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@valeriupredoi valeriupredoi left a comment

Choose a reason for hiding this comment

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

Cheers, Manu - small change (if it's >=8.6, it'll never be 8.1 😁 ) but, more importantly, has Bouwe's question been answered?

environment.yml Outdated Show resolved Hide resolved
Copy link
Contributor

@valeriupredoi valeriupredoi left a comment

Choose a reason for hiding this comment

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

👌 🍺

@valeriupredoi valeriupredoi merged commit 4ca7198 into main Jun 3, 2024
6 checks passed
@valeriupredoi valeriupredoi deleted the unpin_esmpy branch June 3, 2024 13:10
@valeriupredoi
Copy link
Contributor

@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 🍺

@valeriupredoi
Copy link
Contributor

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)

@ehogan
Copy link
Contributor

ehogan commented Jun 26, 2024

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 ESMValGroup/ESMValTool#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 ESMValGroup/ESMValTool#3643 (from 3 weeks ago!) and cherry pick everything after that into the release branch 😭

What would be best? 🤔

@valeriupredoi
Copy link
Contributor

valeriupredoi commented Jun 26, 2024

@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 😆

@bouweandela
Copy link
Member

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?

@bouweandela
Copy link
Member

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

@ehogan
Copy link
Contributor

ehogan commented Jun 26, 2024

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
Copy link
Contributor

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 👍

@schlunma
Copy link
Contributor Author

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
Copy link
Contributor

valeriupredoi commented Jun 26, 2024

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 🤞

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

Successfully merging this pull request may close these issues.

4 participants