Patch the reading in of windspeed by james-a-f-ross · Pull Request #2396 · MetOffice/CSET

James Ross (james-a-f-ross) · 2026-08-18T14:38:26Z

Attempt to fix reading of windspeed for lfric (+other models) where uneeded cubes were not being filtered

Contribution checklist

Aim to have all relevant checks ticked off before merging. See the developer's guide for more detail.

Documentation has been updated to reflect change.
New code has tests, and affected old tests have been updated.
All tests and CI checks pass.
Ensured the pull request title is descriptive.
Attributed any Generative AI, such as GitHub Copilot, used in this PR.
Marked the PR as ready to review.

github-actions · 2026-08-19T10:02:31Z

Total coverage: 93% (HTML report)

Name                                                              Stmts   Miss Branch BrPart  Cover
---------------------------------------------------------------------------------------------------
src/CSET/__init__.py                                                105      0     14      0   100%
src/CSET/_common.py                                                 156      0     54      0   100%
src/CSET/cset_workflow/app/fetch_fcst/bin/fetch_data.py             117     28     26      0    78%
src/CSET/cset_workflow/app/fetch_nimrod/bin/fetch_nimrod.py          81      8     28     11    83%
src/CSET/cset_workflow/app/finish_website/bin/finish_website.py      79      2      8      2    95%
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                                        103      1     26      1    98%
src/CSET/graph.py                                                    44      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                                    249      3     72      4    98%
src/CSET/operators/_stash_to_lfric.py                                 3      0      0      0   100%
src/CSET/operators/_utils.py                                        190      8     72      6    95%
src/CSET/operators/ageofair.py                                      142      7     64      5    94%
src/CSET/operators/aggregate.py                                      77      1     22      1    98%
src/CSET/operators/aviation.py                                       61      0     18      0   100%
src/CSET/operators/collapse.py                                      155      8     72      3    93%
src/CSET/operators/constraints.py                                   115      7     50      2    93%
src/CSET/operators/convection.py                                     38      4     10      2    88%
src/CSET/operators/ensembles.py                                      27      0     14      0   100%
src/CSET/operators/feature.py                                        44      0     10      0   100%
src/CSET/operators/filters.py                                        67      2     30      0    98%
src/CSET/operators/fluxes.py                                         41      0     10      0   100%
src/CSET/operators/humidity.py                                      135      0     52      0   100%
src/CSET/operators/imageprocessing.py                                57      0     16      0   100%
src/CSET/operators/mesoscale.py                                      18      0      2      0   100%
src/CSET/operators/misc.py                                          172      1     72      3    98%
src/CSET/operators/plot.py                                         1117    155    412     71    83%
src/CSET/operators/power_spectrum.py                                 98      3     30      3    95%
src/CSET/operators/precipitation.py                                 204      2     92      2    99%
src/CSET/operators/pressure.py                                       41      0     12      0   100%
src/CSET/operators/read.py                                          436     23    184     14    94%
src/CSET/operators/regrid.py                                        147      1     70      3    98%
src/CSET/operators/scoreswrappers.py                                208     16     58      6    91%
src/CSET/operators/temperature.py                                   121      0     32      0   100%
src/CSET/operators/transect.py                                       63      0     24      0   100%
src/CSET/operators/wind.py                                           46      3     10      2    91%
src/CSET/operators/write.py                                          15      0      6      0   100%
src/CSET/recipes/__init__.py                                        104      0     28      0   100%
src/CSET/sample_data/__init__.py                                      0      0      0      0   100%
---------------------------------------------------------------------------------------------------
TOTAL                                                              5045    283   1758    141    93%

James Ross (james-a-f-ross) · 2026-08-19T10:07:36Z

Perhaps a bit hacky but think I have this working now. Basically hijacking the constraint by adding the original varname onto it, then use that to filter the cubes at the end of _compute_winds . would be good to get feedback ... had a few more complex passes on it before realising I could store varname in the constraint...

James Frost (jfrost-mo)

Some suggestions for simplifications. It will also need some tests before it can be merged.

James Frost (jfrost-mo) · 2026-08-19T11:18:47Z

+
+    varname_copy = (
+        list(varname)
+        if isinstance(varname, Iterable) and not isinstance(varname, str)
+        else [varname]
+    )
    if "wind_speed_at_10m" in iter_maybe(varname):
        if isinstance(varname, str):
            varname = [varname]
        varname.extend(["eastward_wind_at_10m", "northward_wind_at_10m"])
        varname.extend(["u_wind_at_10m", "v_wind_at_10m"])


Then since we know varname_copy will be an iterable, how about something like this to save us the extra str check?

varname_copy = iter_maybe(varname) if "wind_speed_at_10m" in varname_copy: varname = list(varname_copy) varname.extend(("eastward_wind_at_10m", "northward_wind_at_10m", "u_wind_at_10m", "v_wind_at_10m"))

James Frost (jfrost-mo) · 2026-08-19T11:20:21Z

+    varname_copy = (
+        list(varname)
+        if isinstance(varname, Iterable) and not isinstance(varname, str)
+        else [varname]
+    )
    if "wind_speed_at_10m" in iter_maybe(varname):


This is basically what iter_maybe does, so we can just use that instead of reimplementing it.

CSET/src/CSET/_common.py

Lines 395 to 399 in c2fb6f9

def iter_maybe(thing) -> Iterable:

"""Ensure thing is Iterable. Strings count as atoms."""

if isinstance(thing, Iterable) and not isinstance(thing, str):

return thing

return (thing,)

Suggested change

varname_copy = (

list(varname)

if isinstance(varname, Iterable) and not isinstance(varname, str)

else [varname]

)

if "wind_speed_at_10m" in iter_maybe(varname):

varname_copy = iter_maybe(varname)

if "wind_speed_at_10m" in varname_copy:

James Frost (jfrost-mo) · 2026-08-19T11:23:56Z

    cubes = iris.load(input_files, constraint, callback=_loading_callback)
    # If required, compute wind_speed from components.
-    cubes = _compute_winds(cubes)
+


Suggested change

James Frost (jfrost-mo) · 2026-08-19T11:25:59Z

    # the cell methods, but it may not be warranted.
    #
    # A check on UM STASH attributes is also conducted to adjust directions.
+    if isinstance(constraint, iris.Constraint):


Suggested change

if isinstance(constraint, iris.Constraint):

if is not None:

Generally more idiomatic to check for the explicit sentinel value rather than the one that might change. For example, a str could be passed in as the constraint, etc.

James Frost (jfrost-mo) · 2026-08-19T12:11:34Z

    u_constr = iris.Constraint("eastward_wind_at_10m")
    v_constr = iris.Constraint("northward_wind_at_10m")


Do we also need to test for u/v_wind_at_10m?

I'm actually not sure. will users use this as a varname? if not, then I don't think so .

James Frost (jfrost-mo) · 2026-08-19T12:17:31Z

We probably want to drop these related file changes from this pull request to keep it more scoped.

don't actually know why these spaces going commited. will revert

James Frost (jfrost-mo) · 2026-08-19T12:24:10Z

This check is entirely redundant, as it is checked on line 931.

James Frost (jfrost-mo) · 2026-08-19T12:26:33Z

Why is this not doing what we want already? In theory it should be reducing it to just the speed cube if we only have the wind components in the input CubeList.

So it works for the UM but not for LFRIC.. lfric already has the speed cube.. also it's split up ? e.g with the cube i'm using.. :

[<iris 'Cube' of eastward_wind_at_10m / (m s-1) (time: 6; grid_latitude: 672; grid_longitude: 672)>, <iris 'Cube' of wind_speed_at_10m / (m s-1) (time: 6; grid_latitude: 672; grid_longitude: 672)>, <iris 'Cube' of northward_wind_at_10m / (m s-1) (time: 6; grid_latitude: 672; grid_longitude: 672)>, <iris 'Cube' of eastward_wind_at_10m / (m s-1) (time: 6; grid_latitude: 672; grid_longitude: 672)>, <iris 'Cube' of wind_speed_at_10m / (m s-1) (time: 6; grid_latitude: 672; grid_longitude: 672)>, <iris 'Cube' of northward_wind_at_10m / (m s-1) (time: 6; grid_latitude: 672; grid_longitude: 672)>, <iris 'Cube' of eastward_wind_at_10m / (m s-1) (time: 6; grid_latitude: 672; grid_longitude: 672)>, <iris 'Cube' of wind_speed_at_10m / (m s-1) (time: 6; grid_latitude: 672; grid_longitude: 672)>, <iris 'Cube' of northward_wind_at_10m / (m s-1) (time: 6; grid_latitude: 672; grid_longitude: 672)>, <iris 'Cube' of eastward_wind_at_10m / (m s-1) (time: 6; grid_latitude: 672; grid_longitude: 672)>, <iris 'Cube' of wind_speed_at_10m / (m s-1) (time: 6; grid_latitude: 672; grid_longitude: 672)>, <iris 'Cube' of northward_wind_at_10m / (m s-1) (time: 6; grid_latitude: 672; grid_longitude: 672)>, <iris 'Cube' of eastward_wind_at_10m / (m s-1) (time: 6; grid_latitude: 672; grid_longitude: 672)>, <iris 'Cube' of wind_speed_at_10m / (m s-1) (time: 6; grid_latitude: 672; grid_longitude: 672)>, <iris 'Cube' of northward_wind_at_10m / (m s-1) (time: 6; grid_latitude: 672; grid_longitude: 672)>, <iris 'Cube' of eastward_wind_at_10m / (m s-1) (time: 6; grid_latitude: 672; grid_longitude: 672)>, <iris 'Cube' of wind_speed_at_10m / (m s-1) (time: 6; grid_latitude: 672; grid_longitude: 672)>, <iris 'Cube' of northward_wind_at_10m / (m s-1) (time: 6; grid_latitude: 672; grid_longitude: 672)>, <iris 'Cube' of eastward_wind_at_10m / (m s-1) (time: 6; grid_latitude: 672; grid_longitude: 672)>, <iris 'Cube' of wind_speed_at_10m / (m s-1) (time: 6; grid_latitude: 672; grid_longitude: 672)>, <iris 'Cube' of northward_wind_at_10m / (m s-1) (time: 6; grid_latitude: 672; grid_longitude: 672)>, <iris 'Cube' of eastward_wind_at_10m / (m s-1) (time: 6; grid_latitude: 672; grid_longitude: 672)>, <iris 'Cube' of wind_speed_at_10m / (m s-1) (time: 6; grid_latitude: 672; grid_longitude: 672)>, <iris 'Cube' of northward_wind_at_10m / (m s-1) (time: 6; grid_latitude: 672; grid_longitude: 672)>]

so that

if len(cubes) == 2: wind_only = True

doesn't work .

Also user may request any permutation of [wind_speed_at_10m, eastward_wind_at_10m, northward_wind_at_10m]

James Frost (jfrost-mo) · 2026-08-19T12:34:08Z

+    filter_windspeed = []
+    if hasattr(constraint, "varname"):
+        if "wind_speed_at_10m" in constraint.varname:
+            filter_windspeed.append("wind_speed_at_10m")
+        if "eastward_wind_at_10m" in constraint.varname:
+            filter_windspeed.extend(["eastward_wind_at_10m", "u_wind_at_10m"])
+        if "northward_wind_at_10m" in constraint.varname:
+            filter_windspeed.extend(["northward_wind_at_10m", "v_wind_at_10m"])
+    return filter_windspeed


Rather than checking each one can we not just return the varname list as the constraint list?

Suggested change

filter_windspeed = []

if hasattr(constraint, "varname"):

if "wind_speed_at_10m" in constraint.varname:

filter_windspeed.append("wind_speed_at_10m")

if "eastward_wind_at_10m" in constraint.varname:

filter_windspeed.extend(["eastward_wind_at_10m", "u_wind_at_10m"])

if "northward_wind_at_10m" in constraint.varname:

filter_windspeed.extend(["northward_wind_at_10m", "v_wind_at_10m"])

return filter_windspeed

if hasattr(constraint, "varname"):

return constraint.varname

else:

return None

Or do we need to restrict it to only the wind related ones? In which case something like this?

filter_windspeed = [] wind_variables = ("wind_speed_at_10m", "eastward_wind_at_10m", "northward_wind_at_10m", "u_wind_at_10m", "v_wind_at_10m") if hasattr(constraint, "varname"): filter_windspeed = [var in constraint.varname if var in wind_variables] return filter_windspeed

good point, I think I misunderstood a bit and only restricted to wind operators. we obviously don't want to do that so have made your suggested change.

James Frost (jfrost-mo) · 2026-08-19T12:36:05Z

@@ -925,6 +944,16 @@ def _compute_winds(cubes: iris.cube.CubeList):
    except (KeyError, AttributeError):
        pass

+    if filter_windspeed and "observed" not in cubes[0].name():
+        filter_windspeed_constraint = iris.Constraint(
+            cube_func=lambda cube: (
+                cube.long_name in filter_windspeed
+                or cube.standard_name in filter_windspeed
+                or cube.var_name in filter_windspeed
+            )
+        )
+        cubes = cubes.extract(filter_windspeed_constraint)


Check to see whether the separate checks in _compute_winds can be merged or simplified. They seem overcomplicated for what is being done, but also perhaps don't handle different variable names.

James Ross (james-a-f-ross) added 10 commits August 18, 2026 14:44

initial pass at fix

76720a2

fixes

03e814f

try to fix tests

1d7c644

try to fix tests

7bf3307

try to fix tests

e865775

try to fix tests

f5bb741

try to fix tests

37c0cba

fixing tests and refactor

a63df9f

make it less hacky

cfe208e

even less hacky...

92d6229

James Ross (james-a-f-ross) requested review from James Frost (jfrost-mo) and James Warner (jwarner8) and removed request for James Warner (jwarner8) August 19, 2026 10:07

James Frost (jfrost-mo) requested changes Aug 19, 2026

View reviewed changes

James Ross (james-a-f-ross) added 5 commits August 19, 2026 14:29

revert change scorseswrapper

d8e06dd

simplifying

db483a5

simplifying

abf02e4

simplifying and fixing

070653d

adding tests + modifying old ones

349afaf

James Ross (james-a-f-ross) requested a review from James Frost (jfrost-mo) August 20, 2026 13:25

	def iter_maybe(thing) -> Iterable:
	"""Ensure thing is Iterable. Strings count as atoms."""
	if isinstance(thing, Iterable) and not isinstance(thing, str):
	return thing
	return (thing,)

		u_constr = iris.Constraint("eastward_wind_at_10m")
		v_constr = iris.Constraint("northward_wind_at_10m")

Uh oh!

Conversation

James Ross (james-a-f-ross) commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution checklist

Uh oh!

github-actions Bot commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

James Ross (james-a-f-ross) commented Aug 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

James Frost (jfrost-mo) left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

James Ross (james-a-f-ross) commented Aug 18, 2026 •

edited

Loading

github-actions Bot commented Aug 19, 2026 •

edited

Loading

James Ross (james-a-f-ross) commented Aug 19, 2026 •

edited

Loading