#648 update cmor version to v3-13-0#696
Merged
mo-jareddrayton merged 8 commits intomainfrom Nov 20, 2025
Merged
Conversation
Collaborator
Author
|
One other thing I noticed during development, if I remove I wasn't sure if this is a bug in one of Is this expected @matthew-mizielinski ? |
Collaborator
I think this is a bug in CMOR, but is resolvable by removing the |
matthew-mizielinski
approved these changes
Nov 20, 2025
Collaborator
matthew-mizielinski
left a comment
There was a problem hiding this comment.
LGTM and all tests pass
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This closes #648
CMOR Update / Environment Changes
nccmpto get the environment to resolve. This resulted in thenccmpversion changing from1.9.1->1.10.0which I wouldn't expect to cause any problems being a minor version change. All other version changes in unpinned packages look reasonable.Code Changes
Most of the awkwardness with this change come from the change in variant label and how the usage of
cmor_dataset_configprovides the function that does the validation.CDDS/mip_convert/mip_convert/configuration/python_config.py
Line 276 in 93029c6
Without redesigning the current implementation we need to provide this function with the mip_era information. As a class variable I can't see any obvious way to do this so I don't see any drawbacks with making
_configsan instance variables which is assigned on initialisation and can be provided the correct context information.CDDS/mip_convert/mip_convert/configuration/python_config.py
Lines 306 to 307 in f53efd3
There is a simpler solution that I tried first which removes the need for adding the
mip_eraargument to the Class and having to pass theplugin_idthrough many functions you can just useself.config["cmor_dataset]["mip_era"]within theUserConfigclass. the problem is this causes some (somewhat minor) tests to fail. In hindsight I'm almost tempted to revert back to my initial solution and try and save what tests I can.Example breaking test. You need the
"cmor_datasetto get the"mip_era"!CDDS/mip_convert/mip_convert/tests/test_configuration/test_python_config.py
Lines 115 to 121 in f53efd3