Modify to multi variable constraint function - #2120
Conversation
Total coverage: 91% (HTML report)Name Stmts Miss Branch BrPart Cover --------------------------------------------------------------------------------------------------- src/CSET/__init__.py 93 2 12 0 98% src/CSET/_common.py 149 0 52 0 100% src/CSET/cset_workflow/app/fetch_fcst/bin/fetch_data.py 115 27 24 0 79% src/CSET/cset_workflow/app/finish_website/bin/finish_website.py 70 0 4 0 100% src/CSET/cset_workflow/app/parbake_recipes/bin/parbake.py 29 0 8 0 100% src/CSET/cset_workflow/app/send_email/bin/send_email.py 25 0 4 0 100% src/CSET/cset_workflow/lib/python/jinja_utils.py 17 0 6 0 100% src/CSET/extract_workflow.py 47 0 16 0 100% src/CSET/graph.py 43 0 14 0 100% src/CSET/operators/__init__.py 89 0 26 0 100% src/CSET/operators/_atmospheric_constants.py 9 0 0 0 100% src/CSET/operators/_colormaps.py 229 4 62 4 97% src/CSET/operators/_stash_to_lfric.py 3 0 0 0 100% src/CSET/operators/_utils.py 183 8 74 6 95% src/CSET/operators/ageofair.py 141 7 64 5 94% src/CSET/operators/aggregate.py 76 1 22 1 98% src/CSET/operators/aviation.py 60 0 18 0 100% src/CSET/operators/collapse.py 154 12 72 5 91% src/CSET/operators/constraints.py 111 7 48 2 93% src/CSET/operators/convection.py 37 4 10 2 87% src/CSET/operators/ensembles.py 27 0 14 0 100% src/CSET/operators/feature.py 41 0 10 0 100% src/CSET/operators/filters.py 66 2 30 0 98% src/CSET/operators/fluxes.py 41 0 10 0 100% src/CSET/operators/humidity.py 139 0 56 0 100% src/CSET/operators/imageprocessing.py 56 0 16 0 100% src/CSET/operators/mesoscale.py 17 0 2 0 100% src/CSET/operators/misc.py 146 0 54 1 99% src/CSET/operators/plot.py 929 169 318 59 78% src/CSET/operators/power_spectrum.py 97 3 30 3 95% src/CSET/operators/precipitation.py 93 0 50 0 100% src/CSET/operators/pressure.py 41 0 12 0 100% src/CSET/operators/read.py 409 38 178 14 89% src/CSET/operators/regrid.py 122 18 64 1 83% src/CSET/operators/scoreswrappers.py 47 6 12 3 85% src/CSET/operators/temperature.py 121 0 32 0 100% src/CSET/operators/transect.py 62 0 24 0 100% src/CSET/operators/wind.py 45 3 10 2 91% src/CSET/operators/write.py 15 0 6 0 100% src/CSET/recipes/__init__.py 101 0 28 0 100% --------------------------------------------------------------------------------------------------- TOTAL 4295 311 1492 108 91% |
Co-authored-by: James Frost <james.frost@metoffice.gov.uk>
Co-authored-by: James Frost <james.frost@metoffice.gov.uk>
…T into multi_variable_constraint
…T into multi_variable_constraint
|
Will propose this PR may not be needed. There are equivalent instances in other recipes of constraining on different variable names/STASH (e.g., but not only #2084). In general, where multiple variables are required, recipes are organised as: (i.e. constrain only via list of variable names), and then subsequent operators have relevant See for example multi_surface_spatial_plot_sequence.yaml which initially reads in 3 variables from all input files, and then filters for each variable in subsequent operators. Could this work for use-case here? Advise we only introduce additional flexibility to operators where clear use-case in mind. |
The modification to this varname constraint function was required for the (at times) quirky nature of the variable names in the Cardington netcdf files. The dataset is not metadata-consistent enough to rely on a single field. This is the point of this PR. The specific order long_name, standard_name, var_name in the code is appropriate. Moving this logic to the yaml script would only move the quirkiness to the recipe, so overall it would become less portable. |
There was a problem hiding this comment.
Thanks for explaining more of rationale - in general, multiple varnames via constraints will be additional functionality and could simplify some recipes. We should ensure not to see (e.g. document) this as 'Cardington specific'.
Some changes to coding and doc style proposed.
Please update branch to fix conflict and consider proposed changes.
Please also fill in Contribution checklist in top PR description box.
Then comfortable to push this one ahead to merge with main.
Co-authored-by: ukmo-huw-lewis <48992503+ukmo-huw-lewis@users.noreply.github.com>
Co-authored-by: ukmo-huw-lewis <48992503+ukmo-huw-lewis@users.noreply.github.com>
Refactor variable name handling in constraints.py to improve readability and maintainability.
ukmo-huw-lewis
left a comment
There was a problem hiding this comment.
Happy to approve.
Latest updates to preserve wind_speed naming and appropriate testing now pass.
|
Dismissing additional review from James Frost (@jfrost-mo) as blocker to merge PR. |
Legacy review comments showing as resolved, and undertaken subsequent review.
Happy to approve.
Contribution checklist
Aim to have all relevant checks ticked off before merging. See the developer's guide for more detail.
rose-suite.conf.examplehas been updated if new diagnostic added.