MOBT-211: mosg__model_run attribute handling in weather symbols #1670
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1670 +/- ##
==========================================
+ Coverage 98.18% 98.20% +0.01%
==========================================
Files 110 110
Lines 10110 10173 +63
==========================================
+ Hits 9927 9990 +63
Misses 183 183
Continue to review full report at Codecov.
|
This will enable the reuse of this functionality, possibly with modifications, for diagnostics that are created after model blending.
It is not possible to create a model_run_attr from previously model blended inputs if they do not contain a model_run attribute. The cycles at which these source models were run has been lost. As such the set_record_run_attr function will raise an exception.
Added tests to cover the invocation of the set_record_run_attr function from the weather symbols and modal symbol plugins.
improver/utilities/cube_checker.py
Outdated
| try: | ||
| cube.coord("blend_time") | ||
| except CoordinateNotFoundError: | ||
| return False | ||
| else: | ||
| return True |
There was a problem hiding this comment.
| try: | |
| cube.coord("blend_time") | |
| except CoordinateNotFoundError: | |
| return False | |
| else: | |
| return True | |
| return True if "blend_time" in [c.name() for c in cube.coords()] else False |
There was a problem hiding this comment.
I love a one liner, but I'm not sure the proposed approach is clearer, so I am going to leave this one.
There was a problem hiding this comment.
You could still do it in four lines instead of six:
if "blend_time" in [c.name() for c in cube.coords()]:
return True
else:
return False
Actually, it could also be return if "blend_time" in [c.name() for c in cube.coords()]
There was a problem hiding this comment.
I'll go with return "blend_time" in [c.name() for c in cube.coords()] as it is clear and concise.
There was a problem hiding this comment.
Thanks @bayliffe 👍
I've added some comments with help from @mspelman07.
There was a problem hiding this comment.
Thanks for the changes @bayliffe 👍
I think that this is fine apart from a couple of typos.
Co-authored-by: gavinevans <gavin.evans@metoffice.gov.uk>
|
|
||
|
|
||
| def set_record_run_attr( | ||
| cubelist: CubeList, record_run_attr: str, model_id_attr: Optional[str] |
There was a problem hiding this comment.
The default value for model_id_attr in ModalWeatherCode is None, which this method now says it won't accept. We either need to not call this method in this instance, or change the typing back to include None.
There was a problem hiding this comment.
Are you referring to the change from Union[str, None] to Optional[str]? The use of Optional[str] where None is allowed is preferred to Union[str, None] by mypy: https://mypy.readthedocs.io/en/stable/kinds_of_types.html#optional-types-and-the-none-type.
There was a problem hiding this comment.
Yes, I was. I didn't know that. Thanks Gavin!
* master: MOBT127 tiny tweak to filter_realizations (metoppv#1682) Mobt 160 ecc masked data (metoppv#1662) Convert test_ManipulateReliabilityTable to pytest (metoppv#1678) Alter spot extract cli (metoppv#1666) Enable site cube input to ConstructReliabilityCalibrationTables (metoppv#1667) Corrects example of a CLI in the Read the Doc documentation (metoppv#1673) MOBT-211: mosg__model_run attribute handling in weather symbols (metoppv#1670) # Conflicts: # improver_tests/wxcode/wxcode/test_WeatherSymbols.py
* master: Remove flawed interpretation of the blend_time coordinate. (#1690) DOC: Removal of unnecessary exclusions in sphinx apidoc build (#1689) MOBT127 tiny tweak to filter_realizations (#1682) Mobt 160 ecc masked data (#1662) Convert test_ManipulateReliabilityTable to pytest (#1678) Alter spot extract cli (#1666) Enable site cube input to ConstructReliabilityCalibrationTables (#1667) Corrects example of a CLI in the Read the Doc documentation (#1673) MOBT-211: mosg__model_run attribute handling in weather symbols (#1670)
…ppv#1670) * Factor out set_record_run_attr function. This will enable the reuse of this functionality, possibly with modifications, for diagnostics that are created after model blending. * Update record_run_attr function to avoid duplicate entries. * Add exception for model blended input without model_run_attr. It is not possible to create a model_run_attr from previously model blended inputs if they do not contain a model_run attribute. The cycles at which these source models were run has been lost. As such the set_record_run_attr function will raise an exception. * Add model_run setting for wxcode. * Add model_run_setting for wxmodal. * Update checksums to match new test data. * Additional tests. Added tests to cover the invocation of the set_record_run_attr function from the weather symbols and modal symbol plugins. * Typo fix. * Add exception for missing model_id_attr in instances where it is required. * Add unit tests for new is_model_blended function. * Review changes. * Blend test tweak. * Review changes. * Update improver/blending/utilities.py Co-authored-by: gavinevans <gavin.evans@metoffice.gov.uk> Co-authored-by: gavinevans <gavin.evans@metoffice.gov.uk>
Addresses https://github.com/metoppv/mo-blue-team/issues/211
This PR adds the capability to pull through model and cycle information in the
mosg__model_runattribute to the weather symbol outputs. The changes affect bothwxcodeandwxcode-modal. Steps taken were:weighted_blend.pyrecord_run_attroptions to the wxcode CLI and pull that through the pluginrecord_run_attroptions to the wxcode-modal CLI and pull that through the pluginThe acceptance test data relating to these CLIs has been updated where necessary, ensuring it is compliant with our metadata standards. See issue for acceptance test data branch.
Testing: