Unpin cf-units

[bouweandela]

Description

Closes #1655. Continuing from #1656.

It would be nice to unpin cf-units, to make working with recent iris versions easier.

---

Before you get started

• ☝ Create an issue to discuss what you are going to do

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.

• 🧪 The new functionality is relevant and scientifically sound
• 🛠 This pull request has a descriptive title and labels
• 🛠 Code is written according to the code quality guidelines
• 🛠 Unit tests have been added
• 🛠 Changes are backward compatible
• 🛠 Any changed dependencies have been added or removed correctly
• 🛠 The list of authors is up to date
• 🛠 All checks below this pull request were successful

---

To help with the number pull requests:

• 🙏 We kindly ask you to review two other open pull requests in this repository

[bouweandela]

There is one remaining test failure, but it looks like it is caused by caching intermediate results on CircleCI. The cache key contains the date and branch, so this this should solve itself by tomorrow. Running an extra check with a different branch name here: https://app.circleci.com/pipelines/github/ESMValGroup/ESMValCore?branch=unpin-cf-units-fix-cache

[bouweandela]

Still failing, will have another look on Thursday..

[zklaus]

I think the issue is that https://github.com/ESMValGroup/ESMValTool_sample_data doesn't contain any data with calendar standard, but only with the equivalent calendar gregorian, that has been deprecated in CF. Consequently, the change in the first commit in this PR means that the test is asking for a non-existent category.

Solutions might be to add standard data to the sample data or to add an alias for the existing gregorian data via cube_dict["standard"] += cube_dict["gregorian"] or similar here.

[valeriupredoi]

whoa that's a hefty one! Cheers, gents! @bouweandela could you pls fire up the GA tests too before merging (and unfire them up once things pass) 🍺

[bouweandela]

I think the issue is that https://github.com/ESMValGroup/ESMValTool_sample_data doesn't contain any data with calendar standard, but only with the equivalent calendar gregorian, that has been deprecated in CF. Consequently, the change in the first commit in this PR means that the test is asking for a non-existent category.

@zklaus Thanks for the pointer! Any idea why tests that are using a fresh installation are passing just fine with the same data?

[bouweandela]

why tests that are using a fresh installation are passing just fine with the same data?

To answer my own question: the tests work fine with newer versions of cf-units, but fail with 3.0.1 which is still used in the test that start from the docker container. I'll try to fix things so the tests pass with both versions.

[codecov]

Codecov Report

Merging #1770 (98d1995) into main (7b91ec8) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1770   +/-   ##
=======================================
  Coverage   91.51%   91.51%           
=======================================
  Files         202      202           
  Lines       10908    10908           
=======================================
  Hits         9982     9982           
  Misses        926      926           

Impacted Files	Coverage Δ
esmvalcore/cmor/check.py	97.76% <ø> (ø)
esmvalcore/preprocessor/_multimodel.py	96.15% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[valeriupredoi]

whoa that's a hefty one! Cheers, gents! @bouweandela could you pls fire up the GA tests too before merging (and unfire them up once things pass) beer

@bouweandela could you pls merge main here, then fire up the GA tests, then fire them off if all goes to plan? Let me know, I can do that too, am not just passing orders 😁

[bouweandela]

@valeriupredoi All tests passed

[valeriupredoi]

thanks a lot @bouweandela 🍺 You sure you don't want to pin the lower end to >3.1 - that'd be harmless and we'd be on the safe way too?

[bouweandela]

You sure you don't want to pin the lower end to >3.1 - that'd be harmless and we'd be on the safe way too?

@zklaus carefully wrote all the tests so they work with both versions, so I just continued with that idea. The nice thing about that is that if someone does git pull main, things will just keep working without them having to install new software. The downside is of course that it makes the code slightly more complicated.

[bouweandela]

@ESMValGroup/technical-lead-development-team Could someone please have a final look at this and merge?

[sloosvel]

Just to be annoying and nitpicky, I did a search on gregorian and the docstrings in _unify_time_coordinate (in the multimodel module) still mention it. Maybe it could be updated.

[bouweandela]

Thanks for checking @sloosvel, I just changed it.

[bouweandela]

OK, now the tests are failing due to ESMValGroup/ESMValTool#2924

[bouweandela]

It would be best to merge #1805 before this pull request.

[sloosvel]

Sine #1805 is merged, I will merge this