Merge pull request #925 from davidhassell/fix-cyclic-subspace-2

Fix for subspacing with cyclic `cf.wi` and `cf.wo` arguments

Author: davidhassell

.pre-commit-config.yaml

@@ -3,7 +3,11 @@
 
 # Excludes for *all* pre-commit hooks as listed below:
 # (these are documentation files, either old ones or aut-generated ones)
-exclude: docs\/3\..*\d\/|docs\/_downloads\/|docs\/.*\/tutorial\.py
+exclude: |
+    (?x)^(
+        recipes-docs/|
+        docs/
+    )
 
 repos:
 

Changelog.rst

@@ -1,3 +1,13 @@
+Version NEXTVERSION
+--------------
+
+**2026-??-??**
+
+* Fix for subspacing with cyclic `cf.wi` and `cf.wo` arguments
+  (https://github.com/NCAS-CMS/cf-python/issues/887)
+
+----
+
 Version 3.19.0
 --------------
 

cf/mixin/fielddomain.py

@@ -16,7 +16,7 @@
     bounds_combination_mode,
     normalize_slice,
 )
-from ..query import Query, wi
+from ..query import Query, wi, wo
 from ..units import Units
 
 logger = logging.getLogger(__name__)
@@ -245,8 +245,8 @@ def _indices(self, config, data_axes, ancillary_mask, kwargs):
                  tuples of domain axis identifier combinations, each
                  of which has of a `Data` object containing the
                  ancillary mask to apply to those domain axes
-                 immediately after the subspace has been created
-                 by the ``'indices'``. This dictionary will always be
+                 immediately after the subspace has been created by
+                 the ``'indices'``. This dictionary will always be
                  empty if the *ancillary_mask* parameter is False.
 
         """
@@ -456,6 +456,30 @@ def _indices(self, config, data_axes, ancillary_mask, kwargs):
                         if debug:
                             logger.debug("  1-d CASE 2:")  # pragma: no cover
 
+                        arg0, arg1 = value.value
+                        if arg0 > arg1:
+                            # Query has swapped operands (i.e. arg0 >
+                            # arg1) => Create a new equivalant Query
+                            # that has arg0 < arg1, for a new
+                            # arg1. E.g. for a period of 360,
+                            # cf.wi(355, 5) is transformed to
+                            # cf.wi(355, 365).
+                            #
+                            # This is done (effectively) by repeatedly
+                            # adding the cyclic period to arg1 until
+                            # it is greater than arg0, taking into
+                            # account any units that have been set.
+                            period = item.period()
+                            value = value.copy()
+                            value.set_condition_units(period.Units)
+                            arg0, arg1 = value.value
+                            n = ((arg0 - arg1) / period).ceil()
+                            arg1 = arg1 + n * period
+                            if value.operator == "wi":
+                                value = wi(arg0, arg1)
+                            else:
+                                value = wo(arg0, arg1)
+
                         size = item.size
                         if item.increasing:
                             anchor = value.value[0]
@@ -2020,12 +2044,18 @@ def cyclic(
 
             # Check for axes that are currently marked as non-cyclic,
             # but are in fact cyclic.
-            if (
-                len(cyclic) < len(self.domain_axes(todict=True))
-                and self.autocyclic()
-            ):
-                cyclic.update(self._cyclic)
-                self._cyclic = cyclic
+            #
+            # Note: We have to do a "dry run" on the 'autocyclic' call
+            #       in the if test in order to prevent corrupting
+            #       self._cyclic in the case that an axis tested by
+            #       autocyclic is already marked as cylcic, but
+            #       nonetheless autocyclic returns False (sounds
+            #       niche, but this really happens!).
+            if len(cyclic) < len(
+                self.domain_axes(todict=True)
+            ) and self.autocyclic(config={"dry_run": True}):
+                self.autocyclic()
+                cyclic = self._cyclic.copy()
 
             return cyclic
 

cf/mixin/propertiesdatabounds.py

@@ -18,7 +18,9 @@
 )
 from ..functions import equivalent as cf_equivalent
 from ..functions import inspect as cf_inspect
-from ..functions import parse_indices
+from ..functions import (
+    parse_indices,
+)
 from ..functions import size as cf_size
 from ..query import Query
 from ..units import Units

cf/test/test_Field.py

@@ -1290,6 +1290,14 @@ def test_Field_indices(self):
         a[..., [0, 1, 6, 7, 8]] = np.ma.masked
         self.assertTrue(cf.functions._numpy_allclose(g.array, a), g.array)
 
+        # Cyclic cf.wi with swapped operands (increasing coords)
+        for q in (cf.wi(315, 45), cf.wi(-45, -675)):
+            indices = f.indices(grid_longitude=q)
+            g = f[indices]
+            self.assertEqual(g.shape, (1, 10, 3))
+            x = g.dimension_coordinate("X").array
+            self.assertTrue((x == [-40, 0, 40]).all())
+
         # wi (decreasing)
         f.flip("X", inplace=True)
 
@@ -1346,6 +1354,14 @@ def test_Field_indices(self):
             (x == [0, 40, 80, 120, 160, 200, 240, 280, 320][::-1]).all()
         )
 
+        # Cyclic cf.wi with swapped operands (decreasing coords)
+        for q in (cf.wi(315, 45), cf.wi(-45, -675)):
+            indices = f.indices(grid_longitude=q)
+            g = f[indices]
+            self.assertEqual(g.shape, (1, 10, 3))
+            x = g.dimension_coordinate("X").array
+            self.assertTrue((x == [40, 0, -40]).all())
+
         # wo
         f = f0.copy()
 
@@ -3055,6 +3071,13 @@ def test_Field_cyclic_iscyclic(self):
         f2.cyclic("X", iscyclic=False)
         self.assertTrue(f2.iscyclic("X"))
 
+        # In the case that autocyclic thinks the axis is not cyclic,
+        # check that calling iscylcic (which calls cyclic) doesn't
+        # change the cyclicity!
+        f2.dimension_coordinate("X").del_bounds()
+        self.assertTrue(f2.iscyclic("X"))
+        self.assertTrue(f2.iscyclic("X"))
+
     def test_Field_is_discrete_axis(self):
         """Test the `is_discrete_axis` Field method."""
         # No discrete axes