Fix non-functional `import ESMF` after `esmpy>=8.4`

[valeriupredoi]

This is @bouweandela 's elegant solution from https://github.com/ESMValGroup/ESMValCore/pull/1876 implemented here so iris-esmf-regrid can support the newer esmpy, and implicitly allow support for Python=3.11, implicitly allowing us to have Python=3.11 too :grin:

Closes https://github.com/SciTools-incubator/iris-esmf-regrid/issues/227

[codecov[bot]]

Codecov Report

Merging #240 (a325499) into main (55b8354) will decrease coverage by 0.28%. The diff coverage is 81.63%.

@@            Coverage Diff             @@
##             main     #240      +/-   ##
==========================================
- Coverage   99.36%   99.08%   -0.28%     
==========================================
  Files          28       28              
  Lines        2829     2853      +24     
==========================================
+ Hits         2811     2827      +16     
- Misses         18       26       +8     

Impacted Files	Coverage Δ
esmf_regrid/experimental/unstructured_regrid.py	93.18% <80.00%> (-4.19%)	:arrow_down:
esmf_regrid/esmf_regridder.py	92.95% <81.81%> (-2.43%)	:arrow_down:
...tests/integration/esmf_regridder/test_Regridder.py	94.59% <81.81%> (-5.41%)	:arrow_down:
esmf_regrid/_esmf_sdo.py	94.40% <82.35%> (-1.08%)	:arrow_down:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[valeriupredoi]

hey guys - this should be ready, but there are two things to look at:

• there are still a whole bunch of instances of ESMF in docstrings - I have not changed those to esmpy since I think they should still reference ESMF, since that's the original package, plus - I feel uneasy to change docs in packages that are not mine, unless there's something obvious to change :grin:
• I am a bit confused as to why the pre-commit test is failing, I ran flake8 in my repo and all is fine :confused:

Cheers :beer:

[stephenworsley]

Thanks for the PR, I'll look into the pre-commit business.

[valeriupredoi]

Thanks for the PR, I'll look into the pre-commit business.

awesome, cheers muchly @stephenworsley :beer:

[stephenworsley]

Looks like this was a case of black and flake8 disagreeing. I've turned off black formatting for these lines of code and that seems to have fixed it.

[valeriupredoi]

great! Sorry about that, I don't use black :black_flag:

[dhohn]

duplicate of the following? https://github.com/SciTools-incubator/iris-esmf-regrid/pull/231

That PR had a different target tho...

[valeriupredoi]

that's indeed what we're after here too - with the added bonus of having docstrings changed with ESMF -> esmpy too. But that's a different branch, as you mention - not sure if the devs would rather open a PR off that branch to main or accept this PR - whatever's best for them methinks :beer:

[stephenworsley]

Okay, so it turns out the situation is actually a bit more complex... So not only has the name changed from ESMF to esmpy, the installation requirements have also changed. You now need to set an environment variable ESMFMKFILE to import esmpy (https://earthsystemmodeling.org/esmpy_doc/release/latest/ESMPy.pdf#section.3.4). You can see the errors you get with this approach in the tests for this PR #241. I'm not sure how to tell what the appropriate ESMFMKFILE would be in the CI or in general so I'm not sure how to solve this one right now...

[dhohn]

Okay, so it turns out the situation is actually a bit more complex... So not only has the name changed from ESMF to esmpy, the installation requirements have also changed. You now need to set an environment variable ESMFMKFILE to import esmpy (https://earthsystemmodeling.org/esmpy_doc/release/latest/ESMPy.pdf#section.3.4). You can see the errors you get with this approach in the tests for this PR #241. I'm not sure how to tell what the appropriate ESMFMKFILE would be in the CI or in general so I'm not sure how to solve this one right now...

Are you installing into an active conda env? If so I had the same issue: https://github.com/conda-forge/esmf-feedstock/issues/91#issuecomment-1350821853

A deactivate + activate of the conda env should do the trick in that case.

[stephenworsley]

Are you installing into an active conda env? If so I had the same issue: conda-forge/esmf-feedstock#91 (comment)

A deactivate + activate of the conda env should do the trick in that case.

Interesting, I'll have to figure out how to do that in the CI

[zklaus]

Is it a problem in the CI? It shouldn't be: The environment will be setup first and then activated. The problem only arises if one is inside an activated environment and then installs a new version of the esmpy package.

[valeriupredoi]

@zklaus is right - also, looking at your CI test config it looks to me like all the stuff is happening in the (base) conda environment, which is not a good idea, one needs to create separate envs on top of the base env. I am not too familiar with Cirrus CI, so I might be wrong though :beer:

[valeriupredoi]

here is an example conda environment file:

---
name: iris-esmf-regrid
channels:
  - conda-forge
  - nodefaults

dependencies:
  - esmpy
  - flake8
  - iris
  - pytest>=3.9,!=6.0.0rc1,!=6.0.0
  - pytest-cov>=2.10.1
  - pytest-env
  - pytest-html!=2.1.0
  - pytest-metadata>=1.5.1
  - pytest-mypy
  - pytest-mock
  - pytest-xdist

I used this to

mamba env create -n iris-esmf-regrid -f environment.yml
conda activate iris-esmf-regrid

then ran a pytest session on this (current) branch - all tests passed (bar one, complaining I am missing some LFRIC sample data in iris), but most importantly, no issues wrt esmpy which is at:

(iris-esmf-regrid) valeriu@valeriu-PORTEGE-Z30-C:~/iris-esmf-regrid$ conda list esmpy
# packages in environment at /home/valeriu/miniconda3/envs/iris-esmf-regrid:
#
# Name                    Version                   Build  Channel
esmpy                     8.4.0           mpi_mpich_py311hf216de5_102    conda-forge

[valeriupredoi]

@stephenworsley here is a PR I made to me own fork (I dodn't want to spam your repo with yet another PR): https://github.com/valeriupredoi/iris-esmf-regrid/pull/1 It folds in a few things:

• changes from here, to update the ESMF/esmpy import and usage
• I added a conda environment file to hold in your dependencies
• I am running your tests in a Github Actions workflow that builds a conda env from the env file, with mamba via Mambaforge, and runs the tests with pytest over two processes (on feature branch, at PR merge point, and nightly)

Tests look good to me, apart from a failed test that needs some iris test data; it's an example that all's fine without the need to initialize that system variable too. Let me know if you guys would want such a feature in your repo, I can PR that here :beer:

[stephenworsley]

With #228 merged in I believe this should be sorted.

[valeriupredoi]

hi @stephenworsley cheers! #228 folds in all the needed changes brought in by this PR too - slightly misleading since that PR is called CI/lockfiles. Should I close this then? :beer:

[stephenworsley]

@valeriupredoi Yeah, the merge order was slightly odd since I wanted to make sure I was testing against the latest version of esmpy. I think this can be closed down now if you're happy with that.

[valeriupredoi]

sure thing! Cheers @stephenworsley - do you guys need any help from me with the new release that'd include the new changes?

[stephenworsley]

@valeriupredoi thanks for the offer, I think we should be fine for the next release at least but I'll let you know if we run into something that needs a little extra help. I was thinking of doing a release after getting #219 in (which ought to make working with ORCA/NEMO grids feasible), I've already got @trexfeathers reviewing this, but feel free to give it a look if this seems relevant to you're work to let us know if this is going in the right direction.

[valeriupredoi]

awesome stuff! Will do, heading there now :beer: Let me know if any help's needed with the conda packaging etc - will be glad to help!