Merge pull request #35 from znicholls/update-temporal-labels

Add support for tminavg and tmaxavg

Author: znicholls

changelog/35.breaking.md

@@ -0,0 +1,7 @@
+Added support for "tminavg" and "tmaxavg" in line with updates to Table F1 of [Taylor et al.](https://docs.google.com/document/d/19jzecgymgiiEsTDzaaqeLP6pTvLT-NzCMaq-wu-QoOc/edit?pli=1&tab=t.0) as at 22 September 2025
+
+If you have a variable with "time4" as a dimension and "time: max" ("time: min") in the cell methods, the temporal label is now "tmaxavg" ("tminavg") rather than "ti" as it was previously.
+
+In addition, if you have "time: max", "time: min" or "time: sum" in your cell methods
+but do not have a "time" dimension, your temporal label will now be "ti"
+rather than its previously value.

src/cmip_branded_variable_mapper/temporal_label.py

@@ -9,7 +9,7 @@
     DimensionMapper,
 )
 
-TEMPORAL_LABEL_CELL_METHODS_MAPPER = CellMethodsSubStringMapper(
+TEMPORAL_LABEL_CELL_METHODS_INITIAL_TESTS_MAPPER = CellMethodsSubStringMapper(
     sub_string_map={
         "time: max": "tmax",
         "time: min": "tmin",
@@ -18,6 +18,11 @@
 )
 """
 Mapper from sub-strings of cell methods to the temporal label
+
+These are for the 'initial tests'
+i.e. the cell methods which are checked first.
+There is a coupling with dimensions in the logic in
+[get_temporal_label][(m).], be careful!
 """
 
 TEMPORAL_LABEL_DIMENSIONS_MAPPER = DimensionMapper(
@@ -32,14 +37,30 @@
 Mapper from dimensions to the temporal label
 """
 
+TEMPORAL_LABEL_CELL_METHODS_TIME4_TESTS_MAPPER = CellMethodsSubStringMapper(
+    sub_string_map={
+        "time: max": "tmaxavg",
+        "time: min": "tminavg",
+    }
+)
+"""
+Mapper from sub-strings of cell methods to the temporal label for "time4" data
 
-def get_temporal_label(
+I.e. for data which has "time4" as a dimension
+"""
+
+
+def get_temporal_label(  # noqa: PLR0913
     cell_methods: str | None,
     dimensions: tuple[str, ...],
-    cell_methods_mapper: CellMethodsSubStringMapper = (
-        TEMPORAL_LABEL_CELL_METHODS_MAPPER
+    cell_methods_initial_mapper: CellMethodsSubStringMapper = (
+        TEMPORAL_LABEL_CELL_METHODS_INITIAL_TESTS_MAPPER
     ),
+    cell_methods_initial_required_dimension: str = "time",
     dimensions_mapper: DimensionMapper = TEMPORAL_LABEL_DIMENSIONS_MAPPER,
+    cell_methods_time4_mapper: CellMethodsSubStringMapper = (
+        TEMPORAL_LABEL_CELL_METHODS_TIME4_TESTS_MAPPER
+    ),
     fallback: str = "ti",
 ) -> str:
     """
@@ -59,12 +80,29 @@ def get_temporal_label(
     dimensions
         Dimensions of the variable
 
-    cell_methods_mapper
+    cell_methods_initial_mapper
         Mapper to use to get values based on cell methods
 
+        This is for the 'initial' tests
+        i.e. checks of cell methods which are performed
+        before any other checks.
+
+    cell_methods_initial_required_dimension
+        Dimension required for the result of using `cell_methods_initial_mapper`
+        to be returned.
+
+        If this dimension is not found, then the other mapping checks are performed.
+
     dimensions_mapper
         Mapper to use to get values based on dimensions
 
+        If you include "time4" as a mapping here,
+        you can make a mess as you will effectively
+        disable any use of `cell_methods_time4_mapper`.
+
+    cell_methods_time4_mapper
+        Mapper to use to get values based on cell methods if "time4" is in dimensions
+
     fallback
         Value to return if no other conditions are matched
 
@@ -75,10 +113,18 @@ def get_temporal_label(
     """
     if cell_methods is not None:
         # Check cell methods first
-        if (match := cell_methods_mapper.get_value(cell_methods)) is not None:
-            return match
+        if (match := cell_methods_initial_mapper.get_value(cell_methods)) is not None:
+            if cell_methods_initial_required_dimension in dimensions:
+                return match
+
+            # We matched cell methods but not the required dimension,
+            # fall through to other tests
 
     if (match := dimensions_mapper.get_value(dimensions)) is not None:
         return match
 
+    if cell_methods is not None and "time4" in dimensions:
+        if (match := cell_methods_time4_mapper.get_value(cell_methods)) is not None:
+            return match
+
     return fallback

tests/unit/test_map_to_cmip_branded_variable.py

@@ -15,24 +15,39 @@
         pytest.param(
             "area: mean where land time: max",
             ("time", "lat", "lon"),
-            # cell methods wins out over dimension
             "tmax",
             id="tmax",
         ),
+        pytest.param(
+            "area: mean where land time: max",
+            ("lat", "lon"),
+            "ti",
+            id="tmax-missing-dim",
+        ),
         pytest.param(
             "area: mean where land time: min",
             ("time", "lat", "lon"),
-            # cell methods wins out over dimension
             "tmin",
             id="tmin",
         ),
+        pytest.param(
+            "area: mean where land time: min",
+            ("lat", "lon"),
+            "ti",
+            id="tmin-missing-dim",
+        ),
         pytest.param(
             "area: mean where land time: sum",
             ("time", "lat", "lon"),
-            # cell methods wins out over dimension
             "tsum",
             id="tsum",
         ),
+        pytest.param(
+            "area: mean where land time: sum",
+            ("lat", "lon"),
+            "ti",
+            id="tsum-missing-dim",
+        ),
         pytest.param(
             "area: mean where land",
             ("time", "lat", "lon"),
@@ -64,6 +79,19 @@
             id="ti",
         ),
         pytest.param(
+            "area: mean where land time: max",
+            ("time4", "lat", "lon"),
+            "tmaxavg",
+            id="tmaxavg",
+        ),
+        pytest.param(
+            "area: mean where land time: min",
+            ("time4", "lat", "lon"),
+            "tminavg",
+            id="tminavg",
+        ),
+        pytest.param(
+            # Nothing in cell methods therefore ti
             "area: mean where land",
             ("time4", "lat", "lon"),
             "ti",