From ca7f5a1daa5d786e39733d6a44ddcd4639753005 Mon Sep 17 00:00:00 2001 From: David Hassell Date: Fri, 22 Mar 2024 15:41:14 +0000 Subject: [PATCH 1/3] getitem performance --- Changelog.rst | 2 ++ cf/field.py | 18 ++++++++++++++++++ cf/mixin/fielddomain.py | 7 ++++++- cf/test/test_Field.py | 8 ++++++++ 4 files changed, 34 insertions(+), 1 deletion(-) diff --git a/Changelog.rst b/Changelog.rst index c09f1d0802..b2a939d604 100644 --- a/Changelog.rst +++ b/Changelog.rst @@ -11,6 +11,8 @@ version NEXT (https://github.com/NCAS-CMS/cf-python/issues/715) * Improve `cf.Field.collapse` performance by lazily computing reduced axis coordinates (https://github.com/NCAS-CMS/cf-python/issues/741) +* Improve `cf.Field.__getitem__` performance by not re-calculating + axis cyclicity (https://github.com/NCAS-CMS/cf-python/issues/744) * Fix misleading error message when it is not possible to create area weights requested from `cf.Field.collapse` (https://github.com/NCAS-CMS/cf-python/issues/731) diff --git a/cf/field.py b/cf/field.py index fcab8900c8..69e2c0b292 100644 --- a/cf/field.py +++ b/cf/field.py @@ -441,6 +441,23 @@ def __getitem__(self, indices): for axis, size in zip(data_axes, new_data.shape): domain_axes[axis].set_size(size) + # Record which axes were cyclic before the subspace + org_cyclic = [data_axes.index(axis) for axis in new.cyclic()] + + # Se the subspaced data + new.set_data(new_data, axes=data_axes, copy=False) + + # Update axis cylcicity. Note that this can only entail + # setting an originally cyclic axis to be non-cyclic. Doing + # this now it enables us to disable the (possibly very slow) + # automatic check for cyclicity on the 'set_construct' calls + # below. + if org_cyclic: + new_cyclic = new_data.cyclic() + for i in org_cyclic: + if i not in new_cyclic: + new.cyclic(i, iscyclic=False) + # ------------------------------------------------------------ # Subspace constructs with data # ------------------------------------------------------------ @@ -507,6 +524,7 @@ def __getitem__(self, indices): key=key, axes=construct_axes, copy=False, + autocyclic={"no-op": True}, ) new.set_data(new_data, axes=data_axes, copy=False) diff --git a/cf/mixin/fielddomain.py b/cf/mixin/fielddomain.py index 638f9aa855..4ce469eb61 100644 --- a/cf/mixin/fielddomain.py +++ b/cf/mixin/fielddomain.py @@ -1109,10 +1109,15 @@ def autocyclic(self, key=None, coord=None, verbose=None, config={}): :Returns: - `bool` + `bool` or `None` + `True` if the dimension is cycle, `False` if it isn't, + or `None` no checks were done. """ noop = config.get("no-op") + if noop: + # Don't do anything + return if "cyclic" in config: if not config["cyclic"]: diff --git a/cf/test/test_Field.py b/cf/test/test_Field.py index 6f528642fa..13b36bf426 100644 --- a/cf/test/test_Field.py +++ b/cf/test/test_Field.py @@ -663,6 +663,14 @@ def test_Field__getitem__(self): self.assertTrue(np.allclose(f[:, -3:].array, g[:, :3].array)) self.assertTrue(f[:, :4].equals(g[:, 3:])) + # Test setting of axis cyclicity + f.cyclic("grid_longitude", iscyclic=True) + self.assertEqual(f.data.cyclic(), {1}) + g = f[0, :] + self.assertEqual(g.data.cyclic(), {1}) + g = f[:, 0] + self.assertEqual(g.data.cyclic(), set()) + def test_Field__setitem__(self): f = self.f.copy().squeeze() From ade800324168ca61740ad6473c29869cf35cd782 Mon Sep 17 00:00:00 2001 From: David Hassell Date: Sat, 23 Mar 2024 18:50:19 +0000 Subject: [PATCH 2/3] Typos Co-authored-by: Sadie L. Bartholomew --- cf/field.py | 4 ++-- cf/mixin/fielddomain.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/cf/field.py b/cf/field.py index 69e2c0b292..884294c5d2 100644 --- a/cf/field.py +++ b/cf/field.py @@ -444,12 +444,12 @@ def __getitem__(self, indices): # Record which axes were cyclic before the subspace org_cyclic = [data_axes.index(axis) for axis in new.cyclic()] - # Se the subspaced data + # Set the subspaced data new.set_data(new_data, axes=data_axes, copy=False) # Update axis cylcicity. Note that this can only entail # setting an originally cyclic axis to be non-cyclic. Doing - # this now it enables us to disable the (possibly very slow) + # this now enables us to disable the (possibly very slow) # automatic check for cyclicity on the 'set_construct' calls # below. if org_cyclic: diff --git a/cf/mixin/fielddomain.py b/cf/mixin/fielddomain.py index 4ce469eb61..85cf503e55 100644 --- a/cf/mixin/fielddomain.py +++ b/cf/mixin/fielddomain.py @@ -1110,8 +1110,8 @@ def autocyclic(self, key=None, coord=None, verbose=None, config={}): :Returns: `bool` or `None` - `True` if the dimension is cycle, `False` if it isn't, - or `None` no checks were done. + `True` if the dimension is cyclic, `False` if it isn't, + or `None` if no checks were done. """ noop = config.get("no-op") From f696627d38a5315127ba5056e78395a8694b25cd Mon Sep 17 00:00:00 2001 From: David Hassell Date: Sat, 23 Mar 2024 18:50:56 +0000 Subject: [PATCH 3/3] Replace loop with lisyt comprehension Co-authored-by: Sadie L. Bartholomew --- cf/field.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cf/field.py b/cf/field.py index 884294c5d2..7efe404212 100644 --- a/cf/field.py +++ b/cf/field.py @@ -454,9 +454,8 @@ def __getitem__(self, indices): # below. if org_cyclic: new_cyclic = new_data.cyclic() - for i in org_cyclic: - if i not in new_cyclic: - new.cyclic(i, iscyclic=False) + [new.cyclic(i, iscyclic=False) for i in org_cyclic if i not in new_cyclic] + # ------------------------------------------------------------ # Subspace constructs with data