Merge pull request #782 from mattjbr123/fix_cyclic_subset_bug

Fix bug in certain cyclic subsets

Changelog.rst

@@ -8,6 +8,9 @@ version NEXTVERSION
 * Fix bug where `cf.example_fields` returned a `list`
   of Fields rather than a `Fieldlist`
   (https://github.com/NCAS-CMS/cf-python/issues/725)
+* Fix bug where `cf.normalize_slice` doesn't correctly
+  handle certain cyclic slices
+  (https://github.com/NCAS-CMS/cf-python/issues/774)
 
 ----
 

cf/functions.py

@@ -2132,8 +2132,8 @@ def normalize_slice(index, size, cyclic=False):
         return slice(start, stop, step)
 
     if not (
-        (step > 0 and start < 0 and stop > 0)
-        or (step < 0 and start > 0 and stop < 0)
+        (step > 0 and start < 0 and stop >= 0)
+        or (step < 0 and start >= 0 and stop < 0)
     ):
         raise IndexError(
             f"{index!r} is not a {'cyclic ' if cyclic else ''}slice"

cf/test/test_functions.py

@@ -389,7 +389,24 @@ def test_normalize_slice(self):
             cf.normalize_slice(slice(2, 5, -2), 8, cyclic=True),
             slice(2, -3, -2),
         )
-
+
+        self.assertEqual(
+            cf.normalize_slice(slice(-8, 0, 1), 8, cyclic=True),
+            slice(-8, 0, 1)
+            )
+        self.assertEqual(
+            cf.normalize_slice(slice(0, 7, -1), 8, cyclic=True),
+            slice(0, -1, -1)
+            )        
+        self.assertEqual(
+            cf.normalize_slice(slice(-1, -8, 1), 8, cyclic=True),
+            slice(-1, 0, 1)
+            )
+        self.assertEqual(
+            cf.normalize_slice(slice(-8, -1, -1), 8, cyclic=True),
+            slice(0, -1, -1)
+            )        
+
         with self.assertRaises(IndexError):
             cf.normalize_slice([1, 2], 8)
 

docs/source/contributing.rst

@@ -126,3 +126,4 @@ ideas, code, and documentation to the cf library:
 * Sadie Bartholomew
 * Thibault Hallouin
 * Tim Bradshaw
+* Matt Brown