Make CMOR tables configurable through new configuration system and deprecate config-developer.yml #2946
Conversation
bouweandela
force-pushed
the
new-config-cmor
branch
2 times, most recently
from
2b7ae33 to
f7e0982
Compare
bouweandela
added
cmor
bouweandela
force-pushed
the
new-config-cmor
branch
4 times, most recently
from
2403e51 to
c2bd443
Compare
bouweandela
force-pushed
the
new-config-cmor
branch
from
c2bd443 to
5ae3e93
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2946 +/- ##
==========================================
+ Coverage 95.62% 95.70% +0.08%
==========================================
Files 266 266
Lines 15601 15688 +87
==========================================
+ Hits 14918 15014 +96
+ Misses 683 674 -9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Hello, this pull request has been marked with the If you won't be able to finish this in time, don't worry - just unassign the milestone |
…ate default config
|
@bouweandela shout if you need a helping hand here, with the missing tests - I can help. Apart from that, it looks great - and a lot of bookkeeping work too, which is not pleasant 🍻 |
|
Thanks for offering V! The tests shouldn't be too much effort, it's mostly the docs that still need to be updated and I want to think about if I can avoid making #2954 worse. I was actually having a go at fixing that, but realized that it's probably best to leave that for another PR. If you could help out with reviewing and testing that would be great. Hopefully I'll have this ready for review by the end of the week. |
|
sure thing, can do! 🍺 |
| USER_EXTRA_FACETS = Path.home() / ".esmvaltool" / "extra_facets" | ||
|
|
||
|
|
||
| # Set iris.FUTURE flags |
There was a problem hiding this comment.
Moved things out of this module so it can be completely deleted in v2.16.0
| @@ -0,0 +1,1649 @@ | |||
| { | |||
There was a problem hiding this comment.
Generated from the custom CMIP5 style tables using the script
There was a problem hiding this comment.
Pull request overview
This PR modernizes CMOR table configuration by introducing a new configuration system and deprecating the legacy config-developer.yml file. The changes enable separate CMOR table extensions for different CMIP versions and add support for non-CMORized data.
Changes:
- Introduces a new CMOR table configuration system under the
projectsconfiguration with separate CMIP5 and CMIP6 custom table extensions - Deprecates
config-developer.ymland theconfig_developer_fileconfiguration setting - Updates
align_metadatapreprocessor function to support branded variables via newtarget_branding_suffixparameter
Reviewed changes
Copilot reviewed 32 out of 173 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/preprocessor/test_configuration.py | Adds monkeypatch to set config_developer_file for backward compatibility test |
| tests/unit/io/local/test_get_data_sources.py | Updates to use monkeypatch instead of validate_config_developer |
| tests/unit/config/test_data_sources.py | Adds config_developer_file monkeypatch for legacy tests |
| tests/unit/config/test_config_validator.py | Updates validator tests for new CMOR table configuration defaults |
| tests/unit/config/test_config.py | Removes default config_developer_file expectation from tests |
| tests/unit/cmor/test_table.py | Converts unittest to pytest style and fixes var_name test |
| tests/integration/recipe/test_recipe.py | Updates dataset names and expected error messages |
| tests/integration/io/test_local.py | Adds config_developer_file monkeypatch for legacy tests |
| tests/integration/conftest.py | Refactors datafinder fixture to use new LocalDataSource API |
| tests/integration/cmor/test_table.py | Converts to pytest and adds extensive new tests for CMOR table classes |
| tests/integration/cmor/test_read_cmor_tables.py | Adds comprehensive tests for new get_tables function |
| tests/integration/cmor/_fixes/native6/test_era5.py | Removes explicit coordinate long_name assertions |
| esmvalcore/preprocessor/_other.py | Adds target_branding_suffix parameter to align_metadata |
| esmvalcore/io/local.py | Updates debug info formatting when files are found |
| esmvalcore/io/init.py | Fixes docstring format (Arguments → Parameters) |
| esmvalcore/dataset.py | Makes CMOR checks conditional on table availability |
| esmvalcore/config/configurations/defaults/config-user.yml | Removes deprecated config_developer_file option |
| esmvalcore/config/configurations/defaults/cmor_tables.yml | Adds new default CMOR table configurations |
| esmvalcore/config/_config_validators.py | Implements new validation and deprecation for CMOR tables |
| esmvalcore/config/_config.py | Deprecates config-developer module functions |
| esmvalcore/config/init.py | Moves iris.FUTURE flag setting to main config module |
| esmvalcore/cmor/_utils.py | Adds type ignore comment for potential bug |
| esmvalcore/_recipe/check.py | Updates align_metadata check to support branding_suffix |
| esmvalcore/cmor/tables/cmip7/tables/* | Adds new CMIP7 table files |
| esmvalcore/cmor/tables/cmip6-custom/convert-cmip5-to-cmip6.py | Adds conversion script |
| esmvalcore/cmor/tables/cmip5-custom/*.dat | Adds many custom CMIP5-style table files |
| doc/reference/cmor_tables.rst | Updates table path references |
| doc/recipe/preprocessor.rst | Documents new target_branding_suffix parameter |
| doc/quickstart/find_data.rst | Removes hint about custom_cmor_tables |
| doc/quickstart/configure.rst | Extensive updates for new configuration system |
| doc/develop/fixing_data.rst | Updates native dataset configuration instructions |
| doc/changelog.rst | Updates reference to custom CMOR tables |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 33 out of 174 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| """ | ||
| new_coord = generic_level_coord.generic_lev_coords[new_coord_name] | ||
| new_coord = generic_level_coord.generic_lev_coords[new_coord_name] # type: ignore[index] # Is this a bug? |
There was a problem hiding this comment.
There's a type: ignore comment here with a note "Is this a bug?". This should be investigated rather than just ignoring the type error. The comment suggests uncertainty about whether this code is correct.
| new_coord = generic_level_coord.generic_lev_coords[new_coord_name] # type: ignore[index] # Is this a bug? | |
| if new_coord_name is None: | |
| msg = ( | |
| "new_coord_name must not be None when retrieving a new generic " | |
| f"level coordinate for {generic_level_coord_name!r}" | |
| ) | |
| raise ValueError(msg) | |
| new_coord = generic_level_coord.generic_lev_coords[new_coord_name] |
There was a problem hiding this comment.
@schlunma Do you know what the expected behaviour is here? I added type hints, and then this issue showed up.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| ], | ||
| attributes={"comment": COMMENT}, | ||
| ) | ||
| cube.coord("latitude").long_name = "latitude" |
There was a problem hiding this comment.
This was an artifact of using CMIP5 style custom tables. Now all variables in the native6 project are CMIP6 compliant.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 33 out of 175 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
tests/unit/cmor/test_table.py:1
- The comment questions whether
var_namecould be removed, but the test still validates its behavior. Either remove the comment ifvar_nameis intentional, or create a follow-up issue to investigate removing it if it's truly unused.
esmvalcore/config/configurations/defaults/cmor_tables.yml:1 - The comment 'Projects hosted on ESGF' is placed before CMIP7, but CMIP7 is followed by several other projects that are also ESGF-hosted. Consider moving this comment to encompass all relevant ESGF projects or making the scope clearer.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
I'll take it from here, thanks, bud 🍻 I witnessed your toil live, last night, when I was trying to watch a documentary but my phone kept bleeping beeping with your commits 🤣 |
Description
Added features
esmvalcore.preprocessor.align_metadataso it works with branded variablesCloses #2918
Closes #2746
Closes #2364
Links to documentation:
target_branding_suffixtoesmvalcore.preprocessor.align_metadata: https://esmvaltool--2946.org.readthedocs.build/projects/ESMValCore/en/2946/recipe/preprocessor.html#align-metadataDeprecated features
config-developer.ymland the configuration settingconfig_developer_file, upgrade instructions are available here: https://esmvaltool--2946.org.readthedocs.build/projects/ESMValCore/en/2946/quickstart/configure.html#developer-configuration-fileesmvalcore.cmor.table:read_cmor_tableswhich reads the tables based on the deprecated config-developer filecmor_tables_path,default, anddefault_table_prefixarguments to various CMOR table reader classes and the classCustomInfofor reading custom CMOR tables have been deprecated because they are no longer needed with the new configuration formatCMOR_TABLESholding the CMOR tables because of issue Reduce global state and useesmvalcore.config.Sessionobjects instead #2954Before you get started
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: