Clearly separate fixes and CMOR checks

[schlunma]

Description

Current situation

At the moment, automatic fixes to datasets are done in the CMORCheck class via the automatic_fixes keyword. While this weird interconnection is not only conceptually awkward (why would we fix and check at the same time?), this also brings along performance issues since CMOR checks are done twice or even more often (#2154) for a given dataset. Especially for datasets with lots of individual files, this can add up (e.g., I found a 14% decrease in run time for an ICON recipe).

Changes in this PR

This PR disentangles automatic fixes and CMOR checks in a fully backwards-compatible way (apart from the deprecations; see below).

All automatic fixes have been removed from the CMORCheck class and the automatic_fix keyword deprecated (see below). Moreover, a new class GenericFix has been introduced that now takes care of ALL automatic fixes, which is added as a fix for all dataset. The CMOR checks within these fix functions are also deprecated (see below).

Deprecations

• Option automatic_fixes of CMORCheck: use the functions fix_metadata, fix_data, or Dataset.load (which automatically includes the first two functions) instead. CMOR Checks will not include fixes anymore in the future.
• Option check_level in fix_metadata and fix_data: use the functions cmor_check_metadata, cmor_check_data, or cmor_check instead. Fixes will not check the data anymore in the future.

---

Closes #2154

---

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
• 🧪 and 🛠 Documentation is available
• 🛠 Unit tests have been added
• 🛠 Changes are backward compatible But see deprecations above.
• 🛠 The list of authors is up to date
• 🛠 All checks below this pull request were successful Codacy issues can/will not be fixed (the affected functions already had too much arguments).

---

To help with the number pull requests:

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

[codecov]

Codecov Report

Merging #2157 (99c823c) into main (10bd776) will increase coverage by 0.17%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #2157      +/-   ##
==========================================
+ Coverage   93.14%   93.31%   +0.17%     
==========================================
  Files         237      238       +1     
  Lines       12831    13035     +204     
==========================================
+ Hits        11951    12164     +213     
+ Misses        880      871       -9     

Files	Coverage Δ
esmvalcore/cmor/_fixes/cmip6/iitm_esm.py	100.00% <100.00%> (ø)
esmvalcore/cmor/_fixes/cmip6/kace_1_0_g.py	100.00% <100.00%> (ø)
esmvalcore/cmor/_fixes/fix.py	99.72% <100.00%> (+1.13%)	⬆️
esmvalcore/cmor/_utils.py	100.00% <100.00%> (ø)
esmvalcore/cmor/check.py	98.96% <100.00%> (+1.35%)	⬆️
esmvalcore/cmor/fix.py	100.00% <100.00%> (ø)
esmvalcore/cmor/table.py	94.72% <100.00%> (+0.03%)	⬆️
esmvalcore/preprocessor/_time.py	97.72% <100.00%> (ø)

[schlunma]

Thanks @bouweandela for the review! 🚀 Apart from renaming AutomaticFix I think I addressed all your comments. Please let me know what you think.

[bouweandela]

Thanks for making the changes!

[schlunma]

Unfortunately I found an issue with this new implementation. Previously, the fix_metadata of the AutomaticFix class has been applied to a single cube that has been filtered by short_name. Now, to be aligned with the other fix_metadata functions, it operates on cubes.

Since some datasets will always give you multiple cubes when loaded (e.g., the ones with a generic level coordinate will have additional cubes with coordinates like ps when you load them, even if they are fully CMOR-compliant), we get lots of wrong warnings with the new implementation (e.g., about wrong standard_names that are fixed). One could speculate that this is a bug in iris, but that's the current situation.

An easy way to fix this is to build the _get_single_cube function into AutomaticFix.fix_metadata. Any other ideas @bouweandela? I think this new implementation is much nicer than the old want, so I don't want to revert it.

[bouweandela]

An easy way to fix this is to build the _get_single_cube function into AutomaticFix.fix_metadata.

Sounds fine to me, that is quite similar to the current implementation

[bouweandela]

Thanks @schlunma, LGTM!

[valeriupredoi]

Superb work, gents - I can merge now if you guys ready-ready 🍺

[valeriupredoi]

Lovely work 👍