From 5dc661ca97838860103b91dd7869ed8feef7cdf8 Mon Sep 17 00:00:00 2001 From: Patrick Peglar Date: Mon, 31 Oct 2022 12:32:09 +0000 Subject: [PATCH 01/29] Implement split cube attributes. --- lib/iris/cube.py | 207 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 207 insertions(+) diff --git a/lib/iris/cube.py b/lib/iris/cube.py index 8879ade621..3d6a79a040 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -14,6 +14,7 @@ import copy from copy import deepcopy from functools import partial, reduce +import itertools import operator import warnings from xml.dom.minidom import Document @@ -33,6 +34,7 @@ import iris.aux_factory from iris.common import CFVariableMixin, CubeMetadata, metadata_manager_factory from iris.common.metadata import metadata_filter +from iris.common.mixin import LimitedAttributeDict import iris.coord_systems import iris.coords import iris.exceptions @@ -758,6 +760,195 @@ def _is_single_item(testee): return isinstance(testee, str) or not isinstance(testee, Iterable) +class CubeAttrsDict(MutableMapping): + """ + A dict-like object for "Cube.attributes", which provides unified user access to + the combined cube 'local' and 'global' attributes, mimicking the behaviour of a + simple dict. + + This supports all the regular methods of a 'dict', + However, a few things such as the detailed print (repr) are different. + + In addition, the 'locals' and 'globals' properties provide specific access to local + and global attributes, as regular `~iris.common.mixin.LimitedAttributeDict`s. + + Notes + ----- + For type testing, "issubclass(CubeAttrsDict, Mapping)" is True, but + "issubclass(CubeAttrsDict, dict)" is False. + + Properties 'locals' and 'globals' are the two sets of cube attributes. These are + both :class:`~iris.common.mixin.LimitedAttributeDict`. The CubeAttrsDict object + contains *no* additional state of its own, but simply acts as a view on these two. + + When reading (__getitem__, pop. popitem, keys, values etc), it contains all the + keys + values of both 'locals' and 'globals'. When a key occurs in *both* 'locals' + and 'globals', the result is the local value. + + When writing (__setitem__, setdefault, update, etc) to a key already present, the + existing entry in either 'locals' or 'globals' is updated. If both are present, + 'locals' is updated. + + When writing to a new key, this generally updates 'locals'. However, certain + specific names would never normally be 'data' attributes, and these are written as + 'globals' instead. These are determined by Appendix A of the + `_CF Conventions: https://cfconventions.org/` . + At present, these are : 'conventions', 'featureType', 'history', 'title'. + + Examples + -------- + .. doctest:: + :hide: + >>> cube = Cube([0]) + + >>> cube.attributes.update({'history':'fresh', 'x':3}) + >>> print(cube.attributes) + {'history': 'fresh', 'x': 3} + >>> print(repr(cube.attributes)) + CubeAttrsDict(globals={'history': 'fresh'}, locals={'x': 3}) + + >>> cube.attributes['history'] += '+added' + >>> print(repr(cube.attributes)) + CubeAttrsDict(globals={'history': 'fresh+added'}, locals={'x': 3}) + + >>> cube.attributes.locals['history'] = 'per-var' + >>> print(cube.attributes) + {'history': 'per-var', 'x': 3} + >>> print(repr(cube.attributes)) + CubeAttrsDict(globals={'history': 'fresh+added'}, locals={'x': 3, 'history': 'per-var'}) + + """ + + def __init__(self, combined=None, locals=None, globals=None): + # Allow initialisation from a generic dictionary, or local/global specific ones. + # Initialise local + global, defaulting to empty. + # N.B. this is also the way to achieve copy-less construction. + self.locals = locals + self.globals = globals + # Update with combined, if present. + if combined: + # Treat a single input as a 'copy' operation. + # N.B. enforce deep copying, consistent with general Iris usage. + if hasattr(combined, "globals") and hasattr(combined, "locals"): + globals = deepcopy(combined.globals) + locals = deepcopy(combined.locals) + self.globals.update(globals) + self.locals.update(locals) + else: + # Convert an arbitrary single input value to a dict, and update. + combined = deepcopy(dict(combined)) + for key, value in combined.items(): + self[key] = value + + # + # Provide a serialisation interface + # + def __getstate__(self): + return (self.locals, self.globals) + + def __setstate__(self, state): + self.locals, self.globals = state + + # + # Ensure that the stored local/global dictionaries are "LimitedAttributeDicts". + # + @property + def locals(self): + return self._locals + + @locals.setter + def locals(self, attributes): + self._locals = LimitedAttributeDict(attributes or {}) + + @property + def globals(self): + return self._globals + + @globals.setter + def globals(self, attributes): + self._globals = LimitedAttributeDict(attributes or {}) + + # + # The remaining methods are sufficient to generate a complete standard Mapping + # API -- see docs for :class:`collections.abc.MutableMapping`. + # + + def __iter__(self): + # Ordering: all global keys, then all local ones, but omitting duplicates. + # NOTE: this means that in the "summary" view, attributes present in both + # locals+globals are listed first, amongst the globals, even though they appear + # with the *value* from locals. + # Otherwise follows order of insertion, as is normal for dicts. + return itertools.chain( + self.globals.keys(), + (x for x in self.locals.keys() if x not in self.globals), + ) + + def __len__(self): + # Return the number of keys in the 'combined' view. + return len(list(iter(self))) + + def __getitem__(self, key): + # Fetch an item from the "combined attributes". + # If both present, the local setting takes priority. + if key in self.locals: + store = self.locals + else: + store = self.globals + return store[key] + + def __setitem__(self, key, value): + # Assign an attribute value. + # Make local or global according to the existing content, or the known set of + # "normally global" attributes given by CF. + + # If an attribute of this name is already present, update that + # (the local one having priority). + if key in self.locals: + store = self.locals + elif key in self.globals: + store = self.globals + else: + # If NO existing attribute, create local unless it is a "known global" one. + from iris.fileformats.netcdf.saver import _CF_GLOBAL_ATTRS + + # Not in existing + if key in _CF_GLOBAL_ATTRS: + store = self.globals + else: + store = self.locals + + store[key] = value + + def __delitem__(self, key): + """ + Remove an attribute + + Delete from both local + global. + + """ + if key in self.locals: + del self.locals[key] + if key in self.globals: + del self.globals[key] + + def __str__(self): + # Print it just like a "normal" dictionary. + # Convert to a normal dict to do that. + return str(dict(self)) + + def __repr__(self): + # Special repr form, showing "real" contents. + return f"CubeAttrsDict(globals={self.globals}, locals={self.locals})" + + # A copy method is wanted, which is *not* provided by MutableMapping + def copy(self): + # Implement as a deepcopy, consistent with general Iris usage. + globals = deepcopy(self.globals) + locals = deepcopy(self.locals) + return CubeAttrsDict(globals=globals, locals=locals) + + class Cube(CFVariableMixin): """ A single Iris cube of data and metadata. @@ -1012,6 +1203,22 @@ def _names(self): """ return self._metadata_manager._names + # + # Ensure that .attributes is always a :class:`CubeAttrsDict`. + # + @property + def attributes(self): + return self._metadata_manager.attributes + + @attributes.setter + def attributes(self, attributes): + """ + An override to CfVariableMixin.attributes.setter, which ensures that Cube + attributes are stored in a way which distinguishes global + local ones. + + """ + self._metadata_manager.attributes = CubeAttrsDict(attributes) + def _dimensional_metadata(self, name_or_dimensional_metadata): """ Return a single _DimensionalMetadata instance that matches the given From 9bbaf5559db77e271fad6c817e3129877598e8fe Mon Sep 17 00:00:00 2001 From: Patrick Peglar Date: Mon, 31 Oct 2022 15:15:31 +0000 Subject: [PATCH 02/29] Test fixes. --- lib/iris/common/metadata.py | 6 +++- lib/iris/cube.py | 59 +++++++++++++++++++++++++++---------- 2 files changed, 48 insertions(+), 17 deletions(-) diff --git a/lib/iris/common/metadata.py b/lib/iris/common/metadata.py index 8ec39bb4b1..7c9164d771 100644 --- a/lib/iris/common/metadata.py +++ b/lib/iris/common/metadata.py @@ -242,7 +242,11 @@ def __str__(self): field_strings = [] for field in self._fields: value = getattr(self, field) - if value is None or isinstance(value, (str, dict)) and not value: + if ( + value is None + or isinstance(value, (str, Mapping)) + and not value + ): continue field_strings.append(f"{field}={value}") diff --git a/lib/iris/cube.py b/lib/iris/cube.py index 3d6a79a040..20cf8d580e 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -840,15 +840,6 @@ def __init__(self, combined=None, locals=None, globals=None): for key, value in combined.items(): self[key] = value - # - # Provide a serialisation interface - # - def __getstate__(self): - return (self.locals, self.globals) - - def __setstate__(self, state): - self.locals, self.globals = state - # # Ensure that the stored local/global dictionaries are "LimitedAttributeDicts". # @@ -868,6 +859,49 @@ def globals(self): def globals(self, attributes): self._globals = LimitedAttributeDict(attributes or {}) + # + # Provide a serialisation interface + # + def __getstate__(self): + return (self.locals, self.globals) + + def __setstate__(self, state): + self.locals, self.globals = state + + # + # Support simple comparison, even when contents are arrays. + # + def __eq__(self, other): + # Copied from :class:`~iris.common.mixin.LimitedAttributeDict` + match = set(self.keys()) == set(other.keys()) + if match: + for key, value in self.items(): + match = value == other[key] + try: + match = bool(match) + except ValueError: + match = match.all() + if not match: + break + return match + + def __ne__(self, other): + return not self == other + + # + # Provide a copy method, as for 'dict', but *not* provided by MutableMapping + # + def copy(self): + """ + Return a copy. + + Implemented as a deepcopy, consistent with general Iris usage. + + """ + globals = deepcopy(self.globals) + locals = deepcopy(self.locals) + return CubeAttrsDict(globals=globals, locals=locals) + # # The remaining methods are sufficient to generate a complete standard Mapping # API -- see docs for :class:`collections.abc.MutableMapping`. @@ -941,13 +975,6 @@ def __repr__(self): # Special repr form, showing "real" contents. return f"CubeAttrsDict(globals={self.globals}, locals={self.locals})" - # A copy method is wanted, which is *not* provided by MutableMapping - def copy(self): - # Implement as a deepcopy, consistent with general Iris usage. - globals = deepcopy(self.globals) - locals = deepcopy(self.locals) - return CubeAttrsDict(globals=globals, locals=locals) - class Cube(CFVariableMixin): """ From c72c23822385778b6d34715059bec970c45a6af7 Mon Sep 17 00:00:00 2001 From: Patrick Peglar Date: Tue, 1 Nov 2022 12:01:16 +0000 Subject: [PATCH 03/29] Modify examples for simpler metadata printouts. --- docs/src/further_topics/metadata.rst | 15 ++++++++------- lib/iris/cube.py | 5 +++-- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/docs/src/further_topics/metadata.rst b/docs/src/further_topics/metadata.rst index de1afb15af..9d24c681e0 100644 --- a/docs/src/further_topics/metadata.rst +++ b/docs/src/further_topics/metadata.rst @@ -810,16 +810,17 @@ the ``from_metadata`` class method. For example, given the following .. doctest:: metadata-convert - >>> cube.metadata # doctest: +SKIP - CubeMetadata(standard_name='air_temperature', long_name=None, var_name='air_temperature', units=Unit('K'), attributes={'Conventions': 'CF-1.5', 'STASH': STASH(model=1, section=3, item=236), 'Model scenario': 'A1B', 'source': 'Data from Met Office Unified Model 6.05'}, cell_methods=(CellMethod(method='mean', coord_names=('time',), intervals=('6 hour',), comments=()),)) + >>> cube.metadata + CubeMetadata(standard_name='air_temperature', long_name=None, var_name='air_temperature', units=Unit('K'), attributes=CubeAttrsDict(globals={}, locals={'Conventions': 'CF-1.5', 'STASH': STASH(model=1, section=3, item=236), 'Model scenario': 'A1B', 'source': 'Data from Met Office Unified Model 6.05'}), cell_methods=(CellMethod(method='mean', coord_names=('time',), intervals=('6 hour',), comments=()),)) We can easily convert it to a :class:`~iris.common.metadata.DimCoordMetadata` instance using ``from_metadata``, .. doctest:: metadata-convert - >>> DimCoordMetadata.from_metadata(cube.metadata) # doctest: +SKIP - DimCoordMetadata(standard_name='air_temperature', long_name=None, var_name='air_temperature', units=Unit('K'), attributes={'Conventions': 'CF-1.5', 'STASH': STASH(model=1, section=3, item=236), 'Model scenario': 'A1B', 'source': 'Data from Met Office Unified Model 6.05'}, coord_system=None, climatological=None, circular=None) + >>> newmeta = DimCoordMetadata.from_metadata(cube.metadata) + >>> print(newmeta) + DimCoordMetadata(standard_name=air_temperature, var_name=air_temperature, units=K, attributes={'Conventions': 'CF-1.5', 'STASH': STASH(model=1, section=3, item=236), 'Model scenario': 'A1B', 'source': 'Data from Met Office Unified Model 6.05'}) By examining :numref:`metadata members table`, we can see that the :class:`~iris.cube.Cube` and :class:`~iris.coords.DimCoord` container @@ -849,9 +850,9 @@ class instance, .. doctest:: metadata-convert - >>> longitude.metadata.from_metadata(cube.metadata) - DimCoordMetadata(standard_name='air_temperature', long_name=None, var_name='air_temperature', units=Unit('K'), attributes={'Conventions': 'CF-1.5', 'STASH': STASH(model=1, section=3, item=236), 'Model scenario': 'A1B', 'source': 'Data from Met Office Unified Model 6.05'}, coord_system=None, climatological=None, circular=None) - + >>> newmeta = longitude.metadata.from_metadata(cube.metadata) + >>> print(newmeta) + DimCoordMetadata(standard_name=air_temperature, var_name=air_temperature, units=K, attributes={'Conventions': 'CF-1.5', 'STASH': STASH(model=1, section=3, item=236), 'Model scenario': 'A1B', 'source': 'Data from Met Office Unified Model 6.05'}) .. _metadata assignment: diff --git a/lib/iris/cube.py b/lib/iris/cube.py index 20cf8d580e..47e13df0bb 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -770,7 +770,7 @@ class CubeAttrsDict(MutableMapping): However, a few things such as the detailed print (repr) are different. In addition, the 'locals' and 'globals' properties provide specific access to local - and global attributes, as regular `~iris.common.mixin.LimitedAttributeDict`s. + and global attributes, as regular :class:`~iris.common.mixin.LimitedAttributeDict`s. Notes ----- @@ -790,7 +790,7 @@ class CubeAttrsDict(MutableMapping): 'locals' is updated. When writing to a new key, this generally updates 'locals'. However, certain - specific names would never normally be 'data' attributes, and these are written as + specific names would never normally be 'data' attributes, and these are created as 'globals' instead. These are determined by Appendix A of the `_CF Conventions: https://cfconventions.org/` . At present, these are : 'conventions', 'featureType', 'history', 'title'. @@ -830,6 +830,7 @@ def __init__(self, combined=None, locals=None, globals=None): # Treat a single input as a 'copy' operation. # N.B. enforce deep copying, consistent with general Iris usage. if hasattr(combined, "globals") and hasattr(combined, "locals"): + # Copy a mapping with globals/locals, like another 'CubeAttrsDict' globals = deepcopy(combined.globals) locals = deepcopy(combined.locals) self.globals.update(globals) From 839f559ca036c38d4a941b7300398bb40bb2b50c Mon Sep 17 00:00:00 2001 From: Patrick Peglar Date: Thu, 3 Nov 2022 00:32:26 +0000 Subject: [PATCH 04/29] Added tests, small behaviour fixes. --- lib/iris/cube.py | 81 +++-- lib/iris/tests/unit/cube/test_Cube.py | 28 +- .../tests/unit/cube/test_CubeAttrsDict.py | 284 ++++++++++++++++++ 3 files changed, 359 insertions(+), 34 deletions(-) create mode 100644 lib/iris/tests/unit/cube/test_CubeAttrsDict.py diff --git a/lib/iris/cube.py b/lib/iris/cube.py index 47e13df0bb..60fe704cb6 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -10,12 +10,19 @@ """ from collections import OrderedDict -from collections.abc import Container, Iterable, Iterator, MutableMapping import copy from copy import deepcopy from functools import partial, reduce import itertools import operator +from typing import ( + Container, + Iterable, + Iterator, + Mapping, + MutableMapping, + Optional, +) import warnings from xml.dom.minidom import Document import zlib @@ -819,38 +826,56 @@ class CubeAttrsDict(MutableMapping): """ - def __init__(self, combined=None, locals=None, globals=None): + def __init__( + self, + combined: Optional[Mapping] = "__unset", + locals: Optional[Mapping] = None, + globals: Optional[Mapping] = None, + ): # Allow initialisation from a generic dictionary, or local/global specific ones. - # Initialise local + global, defaulting to empty. - # N.B. this is also the way to achieve copy-less construction. + # First initialise locals + globals, defaulting to empty. self.locals = locals self.globals = globals # Update with combined, if present. - if combined: - # Treat a single input as a 'copy' operation. + if not isinstance(combined, str) or combined != "__unset": + # Treat a single input with 'locals' and 'globals' properties as an + # existing CubeAttrsDict, and update from its content. # N.B. enforce deep copying, consistent with general Iris usage. if hasattr(combined, "globals") and hasattr(combined, "locals"): # Copy a mapping with globals/locals, like another 'CubeAttrsDict' - globals = deepcopy(combined.globals) - locals = deepcopy(combined.locals) - self.globals.update(globals) - self.locals.update(locals) + self.globals.update(deepcopy(combined.globals)) + self.locals.update(deepcopy(combined.locals)) else: - # Convert an arbitrary single input value to a dict, and update. - combined = deepcopy(dict(combined)) - for key, value in combined.items(): - self[key] = value + # Treat any arbitrary single input value as a mapping (dict), and + # update from it. + self.update(dict(deepcopy(combined))) # # Ensure that the stored local/global dictionaries are "LimitedAttributeDicts". # + @staticmethod + def _normalise_attrs( + attributes: Optional[Mapping], + ) -> LimitedAttributeDict: + # Convert an input attributes arg into a standard form. + # N.B. content is always a LimitedAttributeDict, and a deep copy of input. + # Allow arg of None, etc. + if not attributes: + attributes = {} + else: + attributes = deepcopy(attributes) + + # Ensure the expected mapping type. + attributes = LimitedAttributeDict(attributes) + return attributes + @property def locals(self): return self._locals @locals.setter def locals(self, attributes): - self._locals = LimitedAttributeDict(attributes or {}) + self._locals = self._normalise_attrs(attributes) @property def globals(self): @@ -858,7 +883,7 @@ def globals(self): @globals.setter def globals(self, attributes): - self._globals = LimitedAttributeDict(attributes or {}) + self._globals = self._normalise_attrs(attributes) # # Provide a serialisation interface @@ -873,18 +898,10 @@ def __setstate__(self, state): # Support simple comparison, even when contents are arrays. # def __eq__(self, other): - # Copied from :class:`~iris.common.mixin.LimitedAttributeDict` - match = set(self.keys()) == set(other.keys()) - if match: - for key, value in self.items(): - match = value == other[key] - try: - match = bool(match) - except ValueError: - match = match.all() - if not match: - break - return match + # For equality, require both globals + locals to match + other = CubeAttrsDict(other) + result = self.locals == other.locals and self.globals == other.globals + return result def __ne__(self, other): return not self == other @@ -896,12 +913,10 @@ def copy(self): """ Return a copy. - Implemented as a deepcopy, consistent with general Iris usage. + Implemented with deep copying, consistent with general Iris usage. """ - globals = deepcopy(self.globals) - locals = deepcopy(self.locals) - return CubeAttrsDict(globals=globals, locals=locals) + return CubeAttrsDict(globals=self.globals, locals=self.locals) # # The remaining methods are sufficient to generate a complete standard Mapping @@ -1245,7 +1260,7 @@ def attributes(self, attributes): attributes are stored in a way which distinguishes global + local ones. """ - self._metadata_manager.attributes = CubeAttrsDict(attributes) + self._metadata_manager.attributes = CubeAttrsDict(attributes or {}) def _dimensional_metadata(self, name_or_dimensional_metadata): """ diff --git a/lib/iris/tests/unit/cube/test_Cube.py b/lib/iris/tests/unit/cube/test_Cube.py index f38d6ef35d..4fba426da3 100644 --- a/lib/iris/tests/unit/cube/test_Cube.py +++ b/lib/iris/tests/unit/cube/test_Cube.py @@ -31,7 +31,7 @@ CellMethod, DimCoord, ) -from iris.cube import Cube +from iris.cube import Cube, CubeAttrsDict import iris.exceptions from iris.exceptions import ( AncillaryVariableNotFoundError, @@ -2997,5 +2997,31 @@ def test_two_with_same_name_specify_instance(self, cube): assert res == cube.cell_measure("wibble") +class TestAttributesProperty: + def test_attrs_type(self): + # Cube attributes are always of a special dictionary type. + cube = Cube([0], attributes={"a": 1}) + assert type(cube.attributes) == CubeAttrsDict + assert cube.attributes == {"a": 1} + + def test_attrs_remove(self): + # Wiping attributes replaces the stored object + cube = Cube([0], attributes={"a": 1}) + attrs = cube.attributes + cube.attributes = None + assert cube.attributes is not attrs + assert type(cube.attributes) == CubeAttrsDict + assert cube.attributes == {} + + def test_attrs_clear(self): + # Clearing attributes leaves the same object + cube = Cube([0], attributes={"a": 1}) + attrs = cube.attributes + cube.attributes.clear() + assert cube.attributes is attrs + assert type(cube.attributes) == CubeAttrsDict + assert cube.attributes == {} + + if __name__ == "__main__": tests.main() diff --git a/lib/iris/tests/unit/cube/test_CubeAttrsDict.py b/lib/iris/tests/unit/cube/test_CubeAttrsDict.py new file mode 100644 index 0000000000..87112b8618 --- /dev/null +++ b/lib/iris/tests/unit/cube/test_CubeAttrsDict.py @@ -0,0 +1,284 @@ +# Copyright Iris contributors +# +# This file is part of Iris and is released under the LGPL license. +# See COPYING and COPYING.LESSER in the root of the repository for full +# licensing details. +"""Unit tests for the `iris.cube.CubeAttrsDict` class.""" + +import pickle + +import numpy as np +import pytest + +from iris.common.mixin import LimitedAttributeDict +from iris.cube import CubeAttrsDict + + +@pytest.fixture +def sample_attrs() -> CubeAttrsDict: + return CubeAttrsDict( + locals={"a": 1, "z": "this"}, globals={"b": 2, "z": "that"} + ) + + +def check_content(attrs, locals=None, globals=None, matches=None): + # Check a CubeAttrsDict for expected properties. + # If locals/globals are set (including =None), test for equality and not identity. + assert isinstance(attrs, CubeAttrsDict) + attr_locals, attr_globals = attrs.locals, attrs.globals + assert type(attr_locals) == LimitedAttributeDict + assert type(attr_globals) == LimitedAttributeDict + if matches: + locals, globals = matches.locals, matches.globals + + def check(arg, content): + if not arg: + arg = {} + if not isinstance(arg, LimitedAttributeDict): + arg = LimitedAttributeDict(arg) + # N.B. if 'arg' is an actual given LimitedAttributeDict, it is not changed.. + # .. we proceed to ensure that the stored content is equal but NOT the same + assert content == arg + assert content is not arg + + check(locals, attr_locals) + check(globals, attr_globals) + + +class Test___init__: + def test_empty(self): + attrs = CubeAttrsDict() + check_content(attrs, None, None) + + def test_from_combined_dict(self): + attrs = CubeAttrsDict({"q": 3, "history": "something"}) + check_content(attrs, locals={"q": 3}, globals={"history": "something"}) + + def test_from_separate_dicts(self): + locals = {"q": 3} + globals = {"history": "something"} + attrs = CubeAttrsDict(locals=locals, globals=globals) + check_content(attrs, locals=locals, globals=globals) + + def test_from_cubeattrsdict(self, sample_attrs): + result = CubeAttrsDict(sample_attrs) + check_content( + result, locals=sample_attrs.locals, globals=sample_attrs.globals + ) + + def test_from_cubeattrsdict_like(self): + class MyDict: + pass + + mydict = MyDict() + locals, globals = {"a": 1}, {"b": 2} + mydict.locals = locals + mydict.globals = globals + attrs = CubeAttrsDict(mydict) + check_content(attrs, locals=locals, globals=globals) + + +class Test_OddMethods: + def test_pickle(self, sample_attrs): + bytes = pickle.dumps(sample_attrs) + result = pickle.loads(bytes) + check_content(result, matches=sample_attrs) + + def test_clear(self, sample_attrs): + sample_attrs.clear() + check_content(sample_attrs, {}, {}) + + def test_del(self, sample_attrs): + # 'z' is in boht locals+globals. Delete removes both. + assert "z" in sample_attrs.keys() + del sample_attrs["z"] + assert "z" not in sample_attrs.keys() + + def test_copy(self, sample_attrs): + copy = sample_attrs.copy() + assert copy is not sample_attrs + check_content(copy, matches=sample_attrs) + + def test_update(self, sample_attrs): + updated = sample_attrs.copy() + updated.update({"q": 77}) + expected_locals = sample_attrs.locals.copy() + expected_locals["q"] = 77 + check_content( + updated, globals=sample_attrs.globals, locals=expected_locals + ) + + def test_to_dict(self, sample_attrs): + result = dict(sample_attrs) + expected = sample_attrs.globals.copy() + expected.update(sample_attrs.locals) + assert result == expected + + def test_array_copies(self): + array = np.array([3, 2, 1, 4]) + map = {"array": array} + attrs = CubeAttrsDict(map) + check_content(attrs, globals=None, locals=map) + attrs_array = attrs["array"] + assert np.all(attrs_array == array) + assert attrs_array is not array + + def test__str__(self, sample_attrs): + result = str(sample_attrs) + assert result == "{'b': 2, 'z': 'this', 'a': 1}" + + def test__repr__(self, sample_attrs): + result = repr(sample_attrs) + expected = ( + "CubeAttrsDict(" + "globals={'b': 2, 'z': 'that'}, " + "locals={'a': 1, 'z': 'this'})" + ) + assert result == expected + + +class TestEq: + def test_eq_empty(self): + attrs_1 = CubeAttrsDict() + attrs_2 = CubeAttrsDict() + assert attrs_1 == attrs_2 + + def test_eq_nonempty(self, sample_attrs): + attrs_1 = sample_attrs + attrs_2 = sample_attrs.copy() + assert attrs_1 == attrs_2 + + @pytest.mark.parametrize("aspect", ["locals", "globals"]) + def test_ne_missing(self, sample_attrs, aspect): + attrs_1 = sample_attrs + attrs_2 = sample_attrs.copy() + del getattr(attrs_2, aspect)["z"] + assert attrs_1 != attrs_2 + assert attrs_2 != attrs_1 + + @pytest.mark.parametrize("aspect", ["locals", "globals"]) + def test_ne_different(self, sample_attrs, aspect): + attrs_1 = sample_attrs + attrs_2 = sample_attrs.copy() + getattr(attrs_2, aspect)["z"] = 99 + assert attrs_1 != attrs_2 + assert attrs_2 != attrs_1 + + def test_ne_locals_vs_globals(self): + attrs_1 = CubeAttrsDict(locals={"a": 1}) + attrs_2 = CubeAttrsDict(globals={"a": 1}) + assert attrs_1 != attrs_2 + assert attrs_2 != attrs_1 + + def test_eq_dict(self): + # A CubeAttrsDict can be equal to a plain dictionary (which would create it) + vals_dict = {"a": 1, "b": 2, "history": "this"} + attrs = CubeAttrsDict(vals_dict) + assert attrs == vals_dict + assert vals_dict == attrs + + def test_ne_dict_local_global(self): + # Dictionary equivalence fails if the local/global assignments are wrong. + # sample dictionary + vals_dict = {"title": "b"} + # these attrs are *not* the same, because 'title' is global by default + attrs = CubeAttrsDict(locals={"title": "b"}) + assert attrs != vals_dict + assert vals_dict != attrs + + def test_empty_not_none(self): + # An empty CubeAttrsDict is not None, and does not compare to 'None' + # N.B. this for compatibility with the LimitedAttributeDict + attrs = CubeAttrsDict() + assert attrs is not None + with pytest.raises(TypeError, match="iterable"): + # Cannot *compare* to None (or anything non-iterable) + # N.B. not actually testing against None, as it upsets black (!) + attrs == 0 + + def test_empty_eq_iterables(self): + # An empty CubeAttrsDict is "equal" to various empty containers + attrs = CubeAttrsDict() + assert attrs == {} + assert attrs == [] + assert attrs == () + + +class TestDictOrderBehaviour: + def test_ordering(self): + attrs = CubeAttrsDict({"a": 1, "b": 2}) + assert list(attrs.keys()) == ["a", "b"] + # Remove, then reinstate 'a' : it will go to the back + del attrs["a"] + attrs["a"] = 1 + assert list(attrs.keys()) == ["b", "a"] + + def test_globals_locals_ordering(self): + # create attrs with a global attribute set *before* a local one .. + attrs = CubeAttrsDict() + attrs.globals.update(dict(a=1, m=3)) + attrs.locals.update(dict(f=7, z=4)) + # .. and check key order of combined attrs + assert list(attrs.keys()) == ["a", "m", "f", "z"] + # create the "same" thing with locals before globals, *and* different key order + attrs = CubeAttrsDict() + attrs.locals.update(dict(z=4, f=7)) + attrs.globals.update(dict(m=3, a=1)) + # .. this shows that the result is not affected either by alphabetical key + # order, or the order of adding locals/globals + # I.E. result is globals-in-create-order, then locals-in-create-order + assert list(attrs.keys()) == ["m", "a", "z", "f"] + + +class TestSettingBehaviours: + def test_add_localtype(self): + attrs = CubeAttrsDict() + attrs["z"] = 3 + check_content(attrs, locals={"z": 3}) + + def test_add_globaltype(self): + attrs = CubeAttrsDict() + attrs["history"] = "this" + check_content(attrs, globals={"history": "this"}) + + def test_overwrite_local(self): + attrs = CubeAttrsDict(locals={"a": 1}) + attrs["a"] = "this" + check_content(attrs, locals={"a": "this"}) + + def test_overwrite_global(self): + attrs = CubeAttrsDict(globals={"a": 1}) + # Since a global attr exists, it will write that instead of creating a local + attrs["a"] = "this" + check_content(attrs, globals={"a": "this"}) + + def test_overwrite_forced_local(self): + attrs = CubeAttrsDict(locals={"history": 1}) + # The attr remains local, even though it would be created global by default + attrs["history"] = 2 + check_content(attrs, locals={"history": 2}) + + def test_overwrite_forced_global(self): + attrs = CubeAttrsDict(globals={"data": 1}) + # The attr remains global, even though it would be created global by default + attrs["data"] = 2 + check_content(attrs, globals={"data": 2}) + + def test_overwrite_both(self): + attrs = CubeAttrsDict(locals={"z": 1}, globals={"z": 1}) + # Where both exits, it will update the local one + attrs["z"] = 2 + check_content(attrs, locals={"z": 2}, globals={"z": 1}) + + def test_local_global_masking(self, sample_attrs): + # initially, local 'z' masks the global one + assert sample_attrs["z"] == sample_attrs.locals["z"] + # remove local, global will show + del sample_attrs.locals["z"] + assert sample_attrs["z"] == sample_attrs.globals["z"] + # re-set local + sample_attrs.locals["z"] = "new" + assert sample_attrs["z"] == "new" + # change the global, makes no difference + sample_attrs.globals["z"] == "other" + assert sample_attrs["z"] == "new" From 21a8dcff34485e2ab1aa1ab6aabbcc90828de0c5 Mon Sep 17 00:00:00 2001 From: Patrick Peglar Date: Thu, 3 Nov 2022 21:29:59 +0000 Subject: [PATCH 05/29] Simplify copy. --- lib/iris/cube.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/iris/cube.py b/lib/iris/cube.py index 60fe704cb6..1917b33af3 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -916,7 +916,7 @@ def copy(self): Implemented with deep copying, consistent with general Iris usage. """ - return CubeAttrsDict(globals=self.globals, locals=self.locals) + return CubeAttrsDict(self) # # The remaining methods are sufficient to generate a complete standard Mapping From e7681c6e70a9e3942ceb8107c31530ce59d2a5ee Mon Sep 17 00:00:00 2001 From: Patrick Peglar Date: Thu, 3 Nov 2022 21:32:41 +0000 Subject: [PATCH 06/29] Fix doctests. --- docs/src/further_topics/metadata.rst | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/src/further_topics/metadata.rst b/docs/src/further_topics/metadata.rst index 9d24c681e0..2e6ae9b62b 100644 --- a/docs/src/further_topics/metadata.rst +++ b/docs/src/further_topics/metadata.rst @@ -131,7 +131,7 @@ We can easily get all of the associated metadata of the :class:`~iris.cube.Cube` using the ``metadata`` property: >>> cube.metadata - CubeMetadata(standard_name='air_temperature', long_name=None, var_name='air_temperature', units=Unit('K'), attributes={'Conventions': 'CF-1.5', 'STASH': STASH(model=1, section=3, item=236), 'Model scenario': 'A1B', 'source': 'Data from Met Office Unified Model 6.05'}, cell_methods=(CellMethod(method='mean', coord_names=('time',), intervals=('6 hour',), comments=()),)) + CubeMetadata(standard_name='air_temperature', long_name=None, var_name='air_temperature', units=Unit('K'), attributes=CubeAttrsDict(globals={}, locals={'Conventions': 'CF-1.5', 'STASH': STASH(model=1, section=3, item=236), 'Model scenario': 'A1B', 'source': 'Data from Met Office Unified Model 6.05'}), cell_methods=(CellMethod(method='mean', coord_names=('time',), intervals=('6 hour',), comments=()),)) We can also inspect the ``metadata`` of the ``longitude`` :class:`~iris.coords.DimCoord` attached to the :class:`~iris.cube.Cube` in the same way: @@ -601,7 +601,7 @@ Now, let's compare the two above instances and see what ``attributes`` member di .. doctest:: richer-metadata - >>> longitude.metadata.difference(metadata) # doctest: +SKIP + >>> longitude.metadata.difference(metadata) DimCoordMetadata(standard_name=None, long_name=None, var_name=None, units=None, attributes=({'neutral face': '😐'}, {'neutral face': '😜', 'upside-down face': '🙃'}), coord_system=None, climatological=None, circular=None) @@ -675,8 +675,8 @@ For example, consider the following :class:`~iris.common.metadata.CubeMetadata`, .. doctest:: metadata-combine - >>> cube.metadata # doctest: +SKIP - CubeMetadata(standard_name='air_temperature', long_name=None, var_name='air_temperature', units=Unit('K'), attributes={'Conventions': 'CF-1.5', 'STASH': STASH(model=1, section=3, item=236), 'Model scenario': 'A1B', 'source': 'Data from Met Office Unified Model 6.05'}, cell_methods=(CellMethod(method='mean', coord_names=('time',), intervals=('6 hour',), comments=()),)) + >>> cube.metadata + CubeMetadata(standard_name='air_temperature', long_name=None, var_name='air_temperature', units=Unit('K'), attributes=CubeAttrsDict(globals={}, locals={'Conventions': 'CF-1.5', 'STASH': STASH(model=1, section=3, item=236), 'Model scenario': 'A1B', 'source': 'Data from Met Office Unified Model 6.05'}), cell_methods=(CellMethod(method='mean', coord_names=('time',), intervals=('6 hour',), comments=()),)) We can perform the **identity function** by comparing the metadata with itself, @@ -700,8 +700,8 @@ which is replaced with a **different value**, >>> metadata = cube.metadata._replace(standard_name="air_pressure_at_sea_level") >>> metadata != cube.metadata True - >>> metadata.combine(cube.metadata) # doctest: +SKIP - CubeMetadata(standard_name=None, long_name=None, var_name='air_temperature', units=Unit('K'), attributes={'STASH': STASH(model=1, section=3, item=236), 'source': 'Data from Met Office Unified Model 6.05', 'Model scenario': 'A1B', 'Conventions': 'CF-1.5'}, cell_methods=(CellMethod(method='mean', coord_names=('time',), intervals=('6 hour',), comments=()),)) + >>> metadata.combine(cube.metadata) + CubeMetadata(standard_name=None, long_name=None, var_name='air_temperature', units=Unit('K'), attributes={'Model scenario': 'A1B', 'STASH': STASH(model=1, section=3, item=236), 'Conventions': 'CF-1.5', 'source': 'Data from Met Office Unified Model 6.05'}, cell_methods=(CellMethod(method='mean', coord_names=('time',), intervals=('6 hour',), comments=()),)) The ``combine`` method combines metadata by performing a **strict** comparison between each of the associated metadata member values, @@ -979,7 +979,7 @@ Indeed, it's also possible to assign to the ``metadata`` property with a >>> longitude.metadata DimCoordMetadata(standard_name='longitude', long_name=None, var_name='longitude', units=Unit('degrees'), attributes={}, coord_system=GeogCS(6371229.0), climatological=False, circular=False) >>> longitude.metadata = cube.metadata - >>> longitude.metadata # doctest: +SKIP + >>> longitude.metadata DimCoordMetadata(standard_name='air_temperature', long_name=None, var_name='air_temperature', units=Unit('K'), attributes={'Conventions': 'CF-1.5', 'STASH': STASH(model=1, section=3, item=236), 'Model scenario': 'A1B', 'source': 'Data from Met Office Unified Model 6.05'}, coord_system=GeogCS(6371229.0), climatological=False, circular=False) Note that, only **common** metadata members will be assigned new associated From cc8e88034a60d7e1a423f2aeff93eb282614f1c3 Mon Sep 17 00:00:00 2001 From: Patrick Peglar Date: Thu, 3 Nov 2022 23:31:49 +0000 Subject: [PATCH 07/29] Skip doctests with non-replicable outputs (from use of sets). --- docs/src/further_topics/metadata.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/src/further_topics/metadata.rst b/docs/src/further_topics/metadata.rst index 2e6ae9b62b..c53cb760a1 100644 --- a/docs/src/further_topics/metadata.rst +++ b/docs/src/further_topics/metadata.rst @@ -601,7 +601,7 @@ Now, let's compare the two above instances and see what ``attributes`` member di .. doctest:: richer-metadata - >>> longitude.metadata.difference(metadata) + >>> longitude.metadata.difference(metadata) # doctest: +SKIP DimCoordMetadata(standard_name=None, long_name=None, var_name=None, units=None, attributes=({'neutral face': '😐'}, {'neutral face': '😜', 'upside-down face': '🙃'}), coord_system=None, climatological=None, circular=None) @@ -700,8 +700,8 @@ which is replaced with a **different value**, >>> metadata = cube.metadata._replace(standard_name="air_pressure_at_sea_level") >>> metadata != cube.metadata True - >>> metadata.combine(cube.metadata) - CubeMetadata(standard_name=None, long_name=None, var_name='air_temperature', units=Unit('K'), attributes={'Model scenario': 'A1B', 'STASH': STASH(model=1, section=3, item=236), 'Conventions': 'CF-1.5', 'source': 'Data from Met Office Unified Model 6.05'}, cell_methods=(CellMethod(method='mean', coord_names=('time',), intervals=('6 hour',), comments=()),)) + >>> metadata.combine(cube.metadata) # doctest: +SKIP + CubeMetadata(standard_name=None, long_name=None, var_name='air_temperature', units=Unit('K'), attributes={'Conventions': 'CF-1.5', 'Model scenario': 'A1B', 'STASH': STASH(model=1, section=3, item=236), 'source': 'Data from Met Office Unified Model 6.05'}, cell_methods=(CellMethod(method='mean', coord_names=('time',), intervals=('6 hour',), comments=()),)) The ``combine`` method combines metadata by performing a **strict** comparison between each of the associated metadata member values, From 7db42ce754e1d6d8b22f69aed2baa3eb4406c250 Mon Sep 17 00:00:00 2001 From: Patrick Peglar Date: Fri, 4 Nov 2022 00:09:42 +0000 Subject: [PATCH 08/29] Tidy test comments, and add extra test. --- .../tests/unit/cube/test_CubeAttrsDict.py | 61 +++++++++++++++---- 1 file changed, 50 insertions(+), 11 deletions(-) diff --git a/lib/iris/tests/unit/cube/test_CubeAttrsDict.py b/lib/iris/tests/unit/cube/test_CubeAttrsDict.py index 87112b8618..29d24fb218 100644 --- a/lib/iris/tests/unit/cube/test_CubeAttrsDict.py +++ b/lib/iris/tests/unit/cube/test_CubeAttrsDict.py @@ -12,6 +12,7 @@ from iris.common.mixin import LimitedAttributeDict from iris.cube import CubeAttrsDict +from iris.fileformats.netcdf.saver import _CF_GLOBAL_ATTRS @pytest.fixture @@ -23,7 +24,7 @@ def sample_attrs() -> CubeAttrsDict: def check_content(attrs, locals=None, globals=None, matches=None): # Check a CubeAttrsDict for expected properties. - # If locals/globals are set (including =None), test for equality and not identity. + # If locals/globals are set, test for equality and non-identity. assert isinstance(attrs, CubeAttrsDict) attr_locals, attr_globals = attrs.locals, attrs.globals assert type(attr_locals) == LimitedAttributeDict @@ -233,13 +234,16 @@ def test_globals_locals_ordering(self): class TestSettingBehaviours: def test_add_localtype(self): attrs = CubeAttrsDict() + # Any attribute not recognised as global should go into 'locals' attrs["z"] = 3 check_content(attrs, locals={"z": 3}) - def test_add_globaltype(self): + @pytest.mark.parametrize("attrname", _CF_GLOBAL_ATTRS) + def test_add_globaltype(self, attrname): + # These specific attributes are recognised as belonging in 'globals' attrs = CubeAttrsDict() - attrs["history"] = "this" - check_content(attrs, globals={"history": "this"}) + attrs[attrname] = "this" + check_content(attrs, globals={attrname: "this"}) def test_overwrite_local(self): attrs = CubeAttrsDict(locals={"a": 1}) @@ -252,21 +256,22 @@ def test_overwrite_global(self): attrs["a"] = "this" check_content(attrs, globals={"a": "this"}) - def test_overwrite_forced_local(self): - attrs = CubeAttrsDict(locals={"history": 1}) - # The attr remains local, even though it would be created global by default - attrs["history"] = 2 - check_content(attrs, locals={"history": 2}) + @pytest.mark.parametrize("global_attrname", _CF_GLOBAL_ATTRS) + def test_overwrite_forced_local(self, global_attrname): + attrs = CubeAttrsDict(locals={global_attrname: 1}) + # The attr *remains* local, even though it would be created global by default + attrs[global_attrname] = 2 + check_content(attrs, locals={global_attrname: 2}) def test_overwrite_forced_global(self): attrs = CubeAttrsDict(globals={"data": 1}) - # The attr remains global, even though it would be created global by default + # The attr remains local, even though it would be created local by default attrs["data"] = 2 check_content(attrs, globals={"data": 2}) def test_overwrite_both(self): attrs = CubeAttrsDict(locals={"z": 1}, globals={"z": 1}) - # Where both exits, it will update the local one + # Where both exist, it will always update the local one attrs["z"] = 2 check_content(attrs, locals={"z": 2}, globals={"z": 1}) @@ -282,3 +287,37 @@ def test_local_global_masking(self, sample_attrs): # change the global, makes no difference sample_attrs.globals["z"] == "other" assert sample_attrs["z"] == "new" + + @pytest.mark.parametrize("globals_or_locals", ("globals", "locals")) + @pytest.mark.parametrize( + "value_type", + ("replace", "emptylist", "emptytuple", "none", "zero", "false"), + ) + def test_replace_subdict(self, globals_or_locals, value_type): + # Writing to attrs.xx always replaces content with a *new* LimitedAttributeDict + locals, globals = {"a": 1}, {"b": 2} + attrs = CubeAttrsDict(locals=locals, globals=globals) + # Snapshot old + write new value, of either locals or globals + old_content = getattr(attrs, globals_or_locals) + value = { + "replace": {"qq": 77}, + "emptytuple": (), + "emptylist": [], + "none": None, + "zero": 0, + "false": False, + }[value_type] + setattr(attrs, globals_or_locals, value) + # check new content is expected type and value + new_content = getattr(attrs, globals_or_locals) + assert isinstance(new_content, LimitedAttributeDict) + assert new_content is not old_content + if value_type != "replace": + value = {} + assert new_content == value + # Check expected whole: i.e. either globals or locals was replaced with value + if globals_or_locals == "globals": + globals = value + else: + locals = value + check_content(attrs, locals=locals, globals=globals) From 7456558e675bd164b6dbdf0203410926aadbe518 Mon Sep 17 00:00:00 2001 From: Patrick Peglar Date: Mon, 17 Jul 2023 17:50:12 +0100 Subject: [PATCH 09/29] Tiny typo. --- lib/iris/cube.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/iris/cube.py b/lib/iris/cube.py index 1917b33af3..aa213040e0 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -788,7 +788,7 @@ class CubeAttrsDict(MutableMapping): both :class:`~iris.common.mixin.LimitedAttributeDict`. The CubeAttrsDict object contains *no* additional state of its own, but simply acts as a view on these two. - When reading (__getitem__, pop. popitem, keys, values etc), it contains all the + When reading (__getitem__, pop, popitem, keys, values etc), it contains all the keys + values of both 'locals' and 'globals'. When a key occurs in *both* 'locals' and 'globals', the result is the local value. From 2687e4f0f29dd1627333137efd74d9d0dfb24531 Mon Sep 17 00:00:00 2001 From: Patrick Peglar Date: Mon, 17 Jul 2023 17:51:24 +0100 Subject: [PATCH 10/29] Remove redundant redefinition of Cube.attributes. --- lib/iris/cube.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/iris/cube.py b/lib/iris/cube.py index aa213040e0..1e96dcf3b7 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -1249,11 +1249,7 @@ def _names(self): # # Ensure that .attributes is always a :class:`CubeAttrsDict`. # - @property - def attributes(self): - return self._metadata_manager.attributes - - @attributes.setter + @CFVariableMixin.attributes.setter def attributes(self, attributes): """ An override to CfVariableMixin.attributes.setter, which ensures that Cube From b055cbea3d1507669335aa814a04e564a5f3ec8d Mon Sep 17 00:00:00 2001 From: Patrick Peglar Date: Tue, 18 Jul 2023 16:58:07 +0100 Subject: [PATCH 11/29] Add CubeAttrsDict in module __all__ + improve docs coverage. --- lib/iris/cube.py | 45 ++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/lib/iris/cube.py b/lib/iris/cube.py index 1e96dcf3b7..642d5f41f5 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -47,7 +47,7 @@ import iris.exceptions import iris.util -__all__ = ["Cube", "CubeList"] +__all__ = ["Cube", "CubeList", "CubeAttrsDict"] # The XML namespace to use for CubeML documents @@ -773,7 +773,7 @@ class CubeAttrsDict(MutableMapping): the combined cube 'local' and 'global' attributes, mimicking the behaviour of a simple dict. - This supports all the regular methods of a 'dict', + This supports all the regular methods of a 'dict'. However, a few things such as the detailed print (repr) are different. In addition, the 'locals' and 'globals' properties provide specific access to local @@ -828,16 +828,51 @@ class CubeAttrsDict(MutableMapping): def __init__( self, - combined: Optional[Mapping] = "__unset", + combined: Optional[Mapping] = "__unspecified", locals: Optional[Mapping] = None, globals: Optional[Mapping] = None, ): - # Allow initialisation from a generic dictionary, or local/global specific ones. + """ + Create a cube attributes dictionary. + + Unlike the attributes of other :class:`CfVariableMixin` subclasses, this is a + "split" dictionary - i.e. it contains separate global + local settings. + + We support initialisation from a generic dictionary (using default global/local + name identification rules), or from specific local and global ones. + + Parameters + ---------- + combined : dict + values to init both 'self.globals' and 'self.locals'. If 'combined' itself + has attributes named 'locals' and 'globals', these are used to update the + respective content (after initially setting the individual ones). + Otherwise, 'combined' is treated as a generic mapping, applied as + ``self.update(combined)``, + i.e. it will set locals and/or globals with the same logic as '__setitem__'. + locals : dict + initial content for 'self.locals' + globals : dict + initial content for 'self.locals' + + Examples + -------- + >>> CubeAttrsDict({'history': 'data-story', 'comment': 'this-cube'}) + CubeAttrsDict(globals={'history': 'data-story'}, locals={'comment': 'this-cube'}) + >>> CubeAttrsDict({'history': 'data-story', 'comment': 'this cube'}).globals + {'history': 'data-story'} + >>> CubeAttrsDict(locals={'history':'local'}) + CubeAttrsDict(globals={}, locals={'history': 'local'}) + >>> CubeAttrsDict(globals={'x': 'global'}, locals={'x':'local'}) + CubeAttrsDict(globals={'x': 'global'}, locals={'x': 'local'}) + >>> + + """ # First initialise locals + globals, defaulting to empty. self.locals = locals self.globals = globals # Update with combined, if present. - if not isinstance(combined, str) or combined != "__unset": + if not isinstance(combined, str) or combined != "__unspecified": # Treat a single input with 'locals' and 'globals' properties as an # existing CubeAttrsDict, and update from its content. # N.B. enforce deep copying, consistent with general Iris usage. From 6e07c8aceb2e107405900fd06ec4f3d1115a3fad Mon Sep 17 00:00:00 2001 From: Patrick Peglar Date: Tue, 18 Jul 2023 16:58:58 +0100 Subject: [PATCH 12/29] Review changes - small test changes. --- lib/iris/tests/unit/cube/test_CubeAttrsDict.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/iris/tests/unit/cube/test_CubeAttrsDict.py b/lib/iris/tests/unit/cube/test_CubeAttrsDict.py index 29d24fb218..2cb490332f 100644 --- a/lib/iris/tests/unit/cube/test_CubeAttrsDict.py +++ b/lib/iris/tests/unit/cube/test_CubeAttrsDict.py @@ -90,7 +90,7 @@ def test_clear(self, sample_attrs): check_content(sample_attrs, {}, {}) def test_del(self, sample_attrs): - # 'z' is in boht locals+globals. Delete removes both. + # 'z' is in both locals+globals. Delete removes both. assert "z" in sample_attrs.keys() del sample_attrs["z"] assert "z" not in sample_attrs.keys() @@ -221,6 +221,8 @@ def test_globals_locals_ordering(self): attrs.locals.update(dict(f=7, z=4)) # .. and check key order of combined attrs assert list(attrs.keys()) == ["a", "m", "f", "z"] + + def test_locals_globals_nonalphabetic_order(self): # create the "same" thing with locals before globals, *and* different key order attrs = CubeAttrsDict() attrs.locals.update(dict(z=4, f=7)) From 53b05d3b53d5b1644034f6746bd25ec724019ae3 Mon Sep 17 00:00:00 2001 From: Patrick Peglar Date: Tue, 18 Jul 2023 17:52:40 +0100 Subject: [PATCH 13/29] More review changes. --- lib/iris/cube.py | 5 +++-- .../tests/unit/cube/test_CubeAttrsDict.py | 20 +++++++++---------- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/lib/iris/cube.py b/lib/iris/cube.py index 642d5f41f5..447e6a1787 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -47,7 +47,7 @@ import iris.exceptions import iris.util -__all__ = ["Cube", "CubeList", "CubeAttrsDict"] +__all__ = ["Cube", "CubeAttrsDict", "CubeList"] # The XML namespace to use for CubeML documents @@ -777,7 +777,8 @@ class CubeAttrsDict(MutableMapping): However, a few things such as the detailed print (repr) are different. In addition, the 'locals' and 'globals' properties provide specific access to local - and global attributes, as regular :class:`~iris.common.mixin.LimitedAttributeDict`s. + and global attributes, as regular + :class:`~iris.common.mixin.LimitedAttributeDict`\\s. Notes ----- diff --git a/lib/iris/tests/unit/cube/test_CubeAttrsDict.py b/lib/iris/tests/unit/cube/test_CubeAttrsDict.py index 2cb490332f..757a0026ce 100644 --- a/lib/iris/tests/unit/cube/test_CubeAttrsDict.py +++ b/lib/iris/tests/unit/cube/test_CubeAttrsDict.py @@ -64,7 +64,7 @@ def test_from_separate_dicts(self): def test_from_cubeattrsdict(self, sample_attrs): result = CubeAttrsDict(sample_attrs) check_content( - result, locals=sample_attrs.locals, globals=sample_attrs.globals + result, matches=sample_attrs ) def test_from_cubeattrsdict_like(self): @@ -248,15 +248,15 @@ def test_add_globaltype(self, attrname): check_content(attrs, globals={attrname: "this"}) def test_overwrite_local(self): - attrs = CubeAttrsDict(locals={"a": 1}) - attrs["a"] = "this" - check_content(attrs, locals={"a": "this"}) + attrs = CubeAttrsDict({"a": 1}) + attrs["a"] = 2 + check_content(attrs, locals={"a": 2}) - def test_overwrite_global(self): - attrs = CubeAttrsDict(globals={"a": 1}) - # Since a global attr exists, it will write that instead of creating a local - attrs["a"] = "this" - check_content(attrs, globals={"a": "this"}) + @pytest.mark.parametrize("attrname", _CF_GLOBAL_ATTRS) + def test_overwrite_global(self, attrname): + attrs = CubeAttrsDict({attrname: 1}) + attrs[attrname] = 2 + check_content(attrs, globals={attrname: 2}) @pytest.mark.parametrize("global_attrname", _CF_GLOBAL_ATTRS) def test_overwrite_forced_local(self, global_attrname): @@ -267,7 +267,7 @@ def test_overwrite_forced_local(self, global_attrname): def test_overwrite_forced_global(self): attrs = CubeAttrsDict(globals={"data": 1}) - # The attr remains local, even though it would be created local by default + # The attr remains global, even though it would be created local by default attrs["data"] = 2 check_content(attrs, globals={"data": 2}) From 7454e224ed3bb09f02ad7a1fe61843eec337896d Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 18 Jul 2023 16:56:11 +0000 Subject: [PATCH 14/29] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- lib/iris/tests/unit/cube/test_CubeAttrsDict.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/iris/tests/unit/cube/test_CubeAttrsDict.py b/lib/iris/tests/unit/cube/test_CubeAttrsDict.py index 757a0026ce..709dc3ccba 100644 --- a/lib/iris/tests/unit/cube/test_CubeAttrsDict.py +++ b/lib/iris/tests/unit/cube/test_CubeAttrsDict.py @@ -63,9 +63,7 @@ def test_from_separate_dicts(self): def test_from_cubeattrsdict(self, sample_attrs): result = CubeAttrsDict(sample_attrs) - check_content( - result, matches=sample_attrs - ) + check_content(result, matches=sample_attrs) def test_from_cubeattrsdict_like(self): class MyDict: From ade712e7f2993c5660866e3ddf20b76873eff70c Mon Sep 17 00:00:00 2001 From: Patrick Peglar Date: Wed, 19 Jul 2023 14:23:45 +0100 Subject: [PATCH 15/29] Fix CubeAttrsDict example docstrings. --- lib/iris/cube.py | 46 +++++++++++++++++++++++++--------------------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/lib/iris/cube.py b/lib/iris/cube.py index 447e6a1787..ee86a4d8c8 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -22,6 +22,7 @@ Mapping, MutableMapping, Optional, + Union ) import warnings from xml.dom.minidom import Document @@ -805,31 +806,30 @@ class CubeAttrsDict(MutableMapping): Examples -------- - .. doctest:: - :hide: - >>> cube = Cube([0]) - >>> cube.attributes.update({'history':'fresh', 'x':3}) + >>> from iris.cube import Cube + >>> cube = Cube([0]) + >>> cube.attributes.update({{"history": "from test-123", "mycode": 3}}) >>> print(cube.attributes) - {'history': 'fresh', 'x': 3} + {{'history': 'from test-123', 'mycode': 3}} >>> print(repr(cube.attributes)) - CubeAttrsDict(globals={'history': 'fresh'}, locals={'x': 3}) + CubeAttrsDict(globals={{'history': 'from test-123'}}, locals={{'mycode': 3}}) - >>> cube.attributes['history'] += '+added' + >>> cube.attributes['history'] += ' +added' >>> print(repr(cube.attributes)) - CubeAttrsDict(globals={'history': 'fresh+added'}, locals={'x': 3}) + CubeAttrsDict(globals={{'history': 'from test-123 +added'}}, locals={{'mycode': 3}}) - >>> cube.attributes.locals['history'] = 'per-var' + >>> cube.attributes.locals['history'] = 'per-variable' >>> print(cube.attributes) - {'history': 'per-var', 'x': 3} + {{'history': 'per-variable', 'mycode': 3}} >>> print(repr(cube.attributes)) - CubeAttrsDict(globals={'history': 'fresh+added'}, locals={'x': 3, 'history': 'per-var'}) + CubeAttrsDict(globals={{'history': 'from test-123 +added'}}, locals={{'mycode': 3, 'history': 'per-variable'}}) """ def __init__( self, - combined: Optional[Mapping] = "__unspecified", + combined: Optional[Union[Mapping, str]] = "__unspecified", locals: Optional[Mapping] = None, globals: Optional[Mapping] = None, ): @@ -858,15 +858,19 @@ def __init__( Examples -------- - >>> CubeAttrsDict({'history': 'data-story', 'comment': 'this-cube'}) - CubeAttrsDict(globals={'history': 'data-story'}, locals={'comment': 'this-cube'}) - >>> CubeAttrsDict({'history': 'data-story', 'comment': 'this cube'}).globals - {'history': 'data-story'} - >>> CubeAttrsDict(locals={'history':'local'}) - CubeAttrsDict(globals={}, locals={'history': 'local'}) - >>> CubeAttrsDict(globals={'x': 'global'}, locals={'x':'local'}) - CubeAttrsDict(globals={'x': 'global'}, locals={'x': 'local'}) - >>> + + >>> from iris.cube import CubeAttrsDict + >>> CubeAttrsDict({'history': 'data-story', 'comment': 'this-cube'}) + CubeAttrsDict(globals={'history': 'data-story'}, locals={'comment': 'this-cube'}) + + >>> CubeAttrsDict({'history': 'data-story', 'comment': 'this cube'}).globals + {'history': 'data-story'} + + >>> CubeAttrsDict(locals={'history': 'local-history'}) + CubeAttrsDict(globals={}, locals={'history': 'local-history'}) + + >>> CubeAttrsDict(globals={'x': 'global'}, locals={'x': 'local'}) + CubeAttrsDict(globals={'x': 'global'}, locals={'x': 'local'}) """ # First initialise locals + globals, defaulting to empty. From 6ebce700f49a0b688e94c3ee979727cd7749c1f4 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 19 Jul 2023 13:25:12 +0000 Subject: [PATCH 16/29] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- lib/iris/cube.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/iris/cube.py b/lib/iris/cube.py index ee86a4d8c8..44c0adf618 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -22,7 +22,7 @@ Mapping, MutableMapping, Optional, - Union + Union, ) import warnings from xml.dom.minidom import Document From f143de066f538c447779e9dc9ff719d59867bb49 Mon Sep 17 00:00:00 2001 From: Patrick Peglar Date: Wed, 19 Jul 2023 14:41:21 +0100 Subject: [PATCH 17/29] Odd small fixes. --- lib/iris/cube.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/iris/cube.py b/lib/iris/cube.py index f7341d5d21..462b94bd85 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -886,7 +886,7 @@ def __init__( locals : dict initial content for 'self.locals' globals : dict - initial content for 'self.locals' + initial content for 'self.globals' Examples -------- @@ -1031,10 +1031,9 @@ def __setitem__(self, key, value): elif key in self.globals: store = self.globals else: - # If NO existing attribute, create local unless it is a "known global" one. + # If NO existing attribute, create local unless it is a "known global" one. from iris.fileformats.netcdf.saver import _CF_GLOBAL_ATTRS - # Not in existing if key in _CF_GLOBAL_ATTRS: store = self.globals else: From e572967a48b3162130a22c8c4d9d491370660ffa Mon Sep 17 00:00:00 2001 From: Patrick Peglar Date: Wed, 19 Jul 2023 15:40:20 +0100 Subject: [PATCH 18/29] Improved docstrings and comments; fix doctests. --- lib/iris/cube.py | 46 +++++++++++++++++++++++++++++++--------------- 1 file changed, 31 insertions(+), 15 deletions(-) diff --git a/lib/iris/cube.py b/lib/iris/cube.py index 462b94bd85..2d265d3e8b 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -825,15 +825,16 @@ class CubeAttrsDict(MutableMapping): When reading (__getitem__, pop, popitem, keys, values etc), it contains all the keys + values of both 'locals' and 'globals'. When a key occurs in *both* 'locals' and 'globals', the result is the local value. + See :meth:``~iris.cube.CubeAttrsDict__getitem__``. When writing (__setitem__, setdefault, update, etc) to a key already present, the existing entry in either 'locals' or 'globals' is updated. If both are present, - 'locals' is updated. + 'locals' is updated. See :meth:``~iris.cube.CubeAttrsDict__setitem__``. When writing to a new key, this generally updates 'locals'. However, certain specific names would never normally be 'data' attributes, and these are created as - 'globals' instead. These are determined by Appendix A of the - `_CF Conventions: https://cfconventions.org/` . + 'globals' instead. These are listed in Appendix A of the + `CF Conventions: `_ . At present, these are : 'conventions', 'featureType', 'history', 'title'. Examples @@ -841,21 +842,21 @@ class CubeAttrsDict(MutableMapping): >>> from iris.cube import Cube >>> cube = Cube([0]) - >>> cube.attributes.update({{"history": "from test-123", "mycode": 3}}) + >>> cube.attributes.update({"history": "from test-123", "mycode": 3}) >>> print(cube.attributes) - {{'history': 'from test-123', 'mycode': 3}} + {'history': 'from test-123', 'mycode': 3} >>> print(repr(cube.attributes)) - CubeAttrsDict(globals={{'history': 'from test-123'}}, locals={{'mycode': 3}}) + CubeAttrsDict(globals={'history': 'from test-123'}, locals={'mycode': 3}) >>> cube.attributes['history'] += ' +added' >>> print(repr(cube.attributes)) - CubeAttrsDict(globals={{'history': 'from test-123 +added'}}, locals={{'mycode': 3}}) + CubeAttrsDict(globals={'history': 'from test-123 +added'}, locals={'mycode': 3}) >>> cube.attributes.locals['history'] = 'per-variable' >>> print(cube.attributes) - {{'history': 'per-variable', 'mycode': 3}} + {'history': 'per-variable', 'mycode': 3} >>> print(repr(cube.attributes)) - CubeAttrsDict(globals={{'history': 'from test-123 +added'}}, locals={{'mycode': 3, 'history': 'per-variable'}}) + CubeAttrsDict(globals={'history': 'from test-123 +added'}, locals={'mycode': 3, 'history': 'per-variable'}) """ @@ -1011,8 +1012,13 @@ def __len__(self): return len(list(iter(self))) def __getitem__(self, key): - # Fetch an item from the "combined attributes". - # If both present, the local setting takes priority. + """ + Fetch an item from the "combined attributes". + + If the name is present in *both* ``self.locals`` and ``self.globals``, then + the local value is returned. + + """ if key in self.locals: store = self.locals else: @@ -1020,10 +1026,20 @@ def __getitem__(self, key): return store[key] def __setitem__(self, key, value): - # Assign an attribute value. - # Make local or global according to the existing content, or the known set of - # "normally global" attributes given by CF. + """ + Assign an attribute value. + If there is *no* existing attribute of this name, then one is created, either + local or global, depending on whether the name is in the known set of "normally + global" attribute names defined by CF. See :class:`~iris.cube.CubeAttrsDict`. + + If there is an *existing* attribute of this name in either ``self.locals`` or + ``self.globals``, then that one is updated (i.e. overwritten). + + If the name is present in *both* locals and globals, then only the local one is + updated. + + """ # If an attribute of this name is already present, update that # (the local one having priority). if key in self.locals: @@ -1043,7 +1059,7 @@ def __setitem__(self, key, value): def __delitem__(self, key): """ - Remove an attribute + Remove an attribute. Delete from both local + global. From 0a5c7e54f72dfca777f7fdd63ea44bf3685bfa17 Mon Sep 17 00:00:00 2001 From: Patrick Peglar Date: Wed, 19 Jul 2023 16:16:06 +0100 Subject: [PATCH 19/29] Don't sidestep netcdf4 thread-safety. --- .../tests/integration/test_netcdf__loadsaveattrs.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/iris/tests/integration/test_netcdf__loadsaveattrs.py b/lib/iris/tests/integration/test_netcdf__loadsaveattrs.py index 77eb3e5324..fa79787950 100644 --- a/lib/iris/tests/integration/test_netcdf__loadsaveattrs.py +++ b/lib/iris/tests/integration/test_netcdf__loadsaveattrs.py @@ -21,13 +21,13 @@ import inspect from typing import Iterable, Optional, Union -import netCDF4 import pytest import iris import iris.coord_systems from iris.cube import Cube import iris.fileformats.netcdf +import iris.fileformats.netcdf._thread_safe_nc as threadsafe_nc4 # First define the known controlled attribute names defined by netCDf and CF conventions # @@ -89,7 +89,7 @@ def _calling_testname(): for frame in stack[1:]: full_name = frame[3] if full_name.startswith("test_"): - # Return the name with the inital "test_" removed. + # Return the name with the initial "test_" removed. test_name = full_name.replace("test_", "") break # Search should not fail, unless we were called from an inappropriate place? @@ -154,7 +154,7 @@ def create_testcase_files( def make_file( filepath: str, global_value=None, var_values=None ) -> str: - ds = netCDF4.Dataset(filepath, "w") + ds = threadsafe_nc4.DatasetWrapper(filepath, "w") if global_value is not None: ds.setncattr(attr_name, global_value) ds.createDimension("x", 3) @@ -257,7 +257,7 @@ def check_roundtrip_results( """ # N.B. there is only ever one result-file, but it can contain various variables # which came from different input files. - ds = netCDF4.Dataset(self.result_filepath) + ds = threadsafe_nc4.DatasetWrapper(self.result_filepath) if global_attr_value is None: assert self.attrname not in ds.ncattrs() else: @@ -849,7 +849,7 @@ def create_save_testcase(self, attr_name, value1, value2=None): # A special case : the stored name is different attr_name = "um_stash_source" try: - ds = netCDF4.Dataset(self.result_filepath) + ds = threadsafe_nc4.DatasetWrapper(self.result_filepath) global_result = ( ds.getncattr(attr_name) if attr_name in ds.ncattrs() else None ) From 16594a28947dc25ae80bb5f4d0ebc5c61db71daf Mon Sep 17 00:00:00 2001 From: Patrick Peglar Date: Wed, 19 Jul 2023 17:41:57 +0100 Subject: [PATCH 20/29] Publicise LimitedAttributeDict, so CubeAttrsDict can refer to it. --- lib/iris/common/mixin.py | 33 ++++++++++++++++--- .../common/mixin/test_LimitedAttributeDict.py | 2 +- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/lib/iris/common/mixin.py b/lib/iris/common/mixin.py index 4c19dd756b..c86132780f 100644 --- a/lib/iris/common/mixin.py +++ b/lib/iris/common/mixin.py @@ -17,7 +17,7 @@ from .metadata import BaseMetadata -__all__ = ["CFVariableMixin"] +__all__ = ["CFVariableMixin", "LimitedAttributeDict"] def _get_valid_standard_name(name): @@ -53,7 +53,29 @@ def _get_valid_standard_name(name): class LimitedAttributeDict(dict): - _forbidden_keys = ( + """ + A specialised 'dict' subclass, which forbids (errors) certain attribute names. + + Used for the attribute dictionaries of all Iris data objects (that is, + :class:`CFVariableMixin` and its subclasses). + + The "excluded" attributes are those which either :mod:`netCDF4` or Iris intpret and + control with special meaning, which therefore should *not* be defined as custom + 'user' attributes on Iris data objects such as cubes. + + For example : "coordinates", "grid_mapping", "scale_factor". + + The 'forbidden' attributes are those listed in + :data:`iris.common.mixin.LimitedAttributeDict.CF_ATTRS_FORBIDDEN` . + + All the forbidden attributes are amongst those listed in + `Appendix A of the CF Conventions: `_ + -- however, not *all* of them, since not all are interpreted by Iris. + + """ + + #: Attributes with special CF meaning, forbidden in Iris attribute dictionaries. + CF_ATTRS_FORBIDDEN = ( "standard_name", "long_name", "units", @@ -78,7 +100,7 @@ def __init__(self, *args, **kwargs): dict.__init__(self, *args, **kwargs) # Check validity of keys for key in self.keys(): - if key in self._forbidden_keys: + if key in self.CF_ATTRS_FORBIDDEN: raise ValueError(f"{key!r} is not a permitted attribute") def __eq__(self, other): @@ -99,11 +121,12 @@ def __ne__(self, other): return not self == other def __setitem__(self, key, value): - if key in self._forbidden_keys: + if key in self.CF_ATTRS_FORBIDDEN: raise ValueError(f"{key!r} is not a permitted attribute") dict.__setitem__(self, key, value) def update(self, other, **kwargs): + """Standard ``dict.update()`` operation.""" # Gather incoming keys keys = [] if hasattr(other, "keys"): @@ -115,7 +138,7 @@ def update(self, other, **kwargs): # Check validity of keys for key in keys: - if key in self._forbidden_keys: + if key in self.CF_ATTRS_FORBIDDEN: raise ValueError(f"{key!r} is not a permitted attribute") dict.update(self, other, **kwargs) diff --git a/lib/iris/tests/unit/common/mixin/test_LimitedAttributeDict.py b/lib/iris/tests/unit/common/mixin/test_LimitedAttributeDict.py index 32c78b6697..84e740e1d8 100644 --- a/lib/iris/tests/unit/common/mixin/test_LimitedAttributeDict.py +++ b/lib/iris/tests/unit/common/mixin/test_LimitedAttributeDict.py @@ -21,7 +21,7 @@ class Test(tests.IrisTest): def setUp(self): - self.forbidden_keys = LimitedAttributeDict._forbidden_keys + self.forbidden_keys = LimitedAttributeDict.CF_ATTRS_FORBIDDEN self.emsg = "{!r} is not a permitted attribute" def test__invalid_keys(self): From 8b070d6fc0d28f2355934398481107b31e9b0796 Mon Sep 17 00:00:00 2001 From: Patrick Peglar Date: Wed, 19 Jul 2023 17:42:24 +0100 Subject: [PATCH 21/29] Fix various internal + external links. --- lib/iris/cube.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/lib/iris/cube.py b/lib/iris/cube.py index 2d265d3e8b..32176fba54 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -819,17 +819,17 @@ class CubeAttrsDict(MutableMapping): "issubclass(CubeAttrsDict, dict)" is False. Properties 'locals' and 'globals' are the two sets of cube attributes. These are - both :class:`~iris.common.mixin.LimitedAttributeDict`. The CubeAttrsDict object + both :class:`~iris.common.mixin.LimitedAttributeDict` . The CubeAttrsDict object contains *no* additional state of its own, but simply acts as a view on these two. When reading (__getitem__, pop, popitem, keys, values etc), it contains all the keys + values of both 'locals' and 'globals'. When a key occurs in *both* 'locals' and 'globals', the result is the local value. - See :meth:``~iris.cube.CubeAttrsDict__getitem__``. + See :meth:`~iris.cube.CubeAttrsDict.__getitem__` . When writing (__setitem__, setdefault, update, etc) to a key already present, the existing entry in either 'locals' or 'globals' is updated. If both are present, - 'locals' is updated. See :meth:``~iris.cube.CubeAttrsDict__setitem__``. + 'locals' is updated. See :meth:`~iris.cube.CubeAttrsDict.__setitem__` . When writing to a new key, this generally updates 'locals'. However, certain specific names would never normally be 'data' attributes, and these are created as @@ -869,8 +869,9 @@ def __init__( """ Create a cube attributes dictionary. - Unlike the attributes of other :class:`CfVariableMixin` subclasses, this is a - "split" dictionary - i.e. it contains separate global + local settings. + Unlike the attributes of other :class:`~iris.common.mixin.CFVariableMixin` + subclasses, this is a "split" dictionary - i.e. it contains separate global + + local settings. We support initialisation from a generic dictionary (using default global/local name identification rules), or from specific local and global ones. @@ -883,7 +884,8 @@ def __init__( respective content (after initially setting the individual ones). Otherwise, 'combined' is treated as a generic mapping, applied as ``self.update(combined)``, - i.e. it will set locals and/or globals with the same logic as '__setitem__'. + i.e. it will set locals and/or globals with the same logic as + :meth:`~iris.cube.CubeAttrsDict.__setitem__` . locals : dict initial content for 'self.locals' globals : dict @@ -993,7 +995,8 @@ def copy(self): # # The remaining methods are sufficient to generate a complete standard Mapping - # API -- see docs for :class:`collections.abc.MutableMapping`. + # API. See - + # https://docs.python.org/3/reference/datamodel.html#emulating-container-types. # def __iter__(self): From bab365e907b00a032e35385516bde45f5361afbc Mon Sep 17 00:00:00 2001 From: Patrick Peglar Date: Thu, 20 Jul 2023 10:11:19 +0100 Subject: [PATCH 22/29] Update lib/iris/cube.py Co-authored-by: Martin Yeo <40734014+trexfeathers@users.noreply.github.com> --- lib/iris/cube.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/iris/cube.py b/lib/iris/cube.py index 32176fba54..c8df4abeee 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -802,7 +802,7 @@ def _is_single_item(testee): class CubeAttrsDict(MutableMapping): """ - A dict-like object for "Cube.attributes", which provides unified user access to + A dict-like object for Cube :attr:`~iris.cube.Cube.attributes`, which provides unified user access to the combined cube 'local' and 'global' attributes, mimicking the behaviour of a simple dict. From 4f22ebad49290e7958081f7e931dda6cb30f967b Mon Sep 17 00:00:00 2001 From: Patrick Peglar Date: Thu, 20 Jul 2023 10:11:40 +0100 Subject: [PATCH 23/29] Update lib/iris/cube.py Co-authored-by: Martin Yeo <40734014+trexfeathers@users.noreply.github.com> --- lib/iris/cube.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/iris/cube.py b/lib/iris/cube.py index c8df4abeee..b06d8f2d77 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -803,7 +803,7 @@ def _is_single_item(testee): class CubeAttrsDict(MutableMapping): """ A dict-like object for Cube :attr:`~iris.cube.Cube.attributes`, which provides unified user access to - the combined cube 'local' and 'global' attributes, mimicking the behaviour of a + the combined cube ``local`` and ``global`` attributes, mimicking the behaviour of a simple dict. This supports all the regular methods of a 'dict'. From 0ab17f47d838449a6f00e85f859191a64e7e5b37 Mon Sep 17 00:00:00 2001 From: Patrick Peglar Date: Thu, 20 Jul 2023 10:12:28 +0100 Subject: [PATCH 24/29] Update lib/iris/cube.py Co-authored-by: Martin Yeo <40734014+trexfeathers@users.noreply.github.com> --- lib/iris/cube.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/iris/cube.py b/lib/iris/cube.py index b06d8f2d77..64f299eb4f 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -806,7 +806,7 @@ class CubeAttrsDict(MutableMapping): the combined cube ``local`` and ``global`` attributes, mimicking the behaviour of a simple dict. - This supports all the regular methods of a 'dict'. + This supports all the regular methods of a :class:`dict`. However, a few things such as the detailed print (repr) are different. In addition, the 'locals' and 'globals' properties provide specific access to local From fc109e9e25f729564b034bf176ed6d93ff2418d4 Mon Sep 17 00:00:00 2001 From: Patrick Peglar Date: Thu, 20 Jul 2023 10:13:16 +0100 Subject: [PATCH 25/29] Update lib/iris/cube.py Co-authored-by: Martin Yeo <40734014+trexfeathers@users.noreply.github.com> --- lib/iris/cube.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/iris/cube.py b/lib/iris/cube.py index 64f299eb4f..38dc05642a 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -815,8 +815,8 @@ class CubeAttrsDict(MutableMapping): Notes ----- - For type testing, "issubclass(CubeAttrsDict, Mapping)" is True, but - "issubclass(CubeAttrsDict, dict)" is False. + For type testing, ``issubclass(CubeAttrsDict, Mapping)`` is ``True``, but + ``issubclass(CubeAttrsDict, dict)`` is ``False``. Properties 'locals' and 'globals' are the two sets of cube attributes. These are both :class:`~iris.common.mixin.LimitedAttributeDict` . The CubeAttrsDict object From 96792e3f60e610dffc184c8c2be12b692000d255 Mon Sep 17 00:00:00 2001 From: Patrick Peglar Date: Thu, 20 Jul 2023 11:56:37 +0100 Subject: [PATCH 26/29] Streamline docs. --- lib/iris/cube.py | 87 +++++++++++++++++++++++------------------------- 1 file changed, 41 insertions(+), 46 deletions(-) diff --git a/lib/iris/cube.py b/lib/iris/cube.py index 38dc05642a..7fbc2e46e5 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -802,41 +802,24 @@ def _is_single_item(testee): class CubeAttrsDict(MutableMapping): """ - A dict-like object for Cube :attr:`~iris.cube.Cube.attributes`, which provides unified user access to - the combined cube ``local`` and ``global`` attributes, mimicking the behaviour of a - simple dict. + A :class:`dict`\\-like object for :attr:`iris.cube.Cube.attributes`, + providing unified user access to combined cube "local" and "global" attributes + dictionaries, with the access behaviour of an ordinary (single) dictionary. - This supports all the regular methods of a :class:`dict`. - However, a few things such as the detailed print (repr) are different. + Properties :attr:`globals` and :attr:`locals` are regular + :class:`~iris.common.mixin.LimitedAttributeDict`\\s, which can be accessed and + modified separately. The :class:`CubeAttrsDict` itself contains *no* additional + state, but simply provides a 'combined' view of both global + local attributes. - In addition, the 'locals' and 'globals' properties provide specific access to local - and global attributes, as regular - :class:`~iris.common.mixin.LimitedAttributeDict`\\s. + All the read- and write-type methods, such as ``get()``, ``update()``, ``values()``, + behave according to the logic documented for : :meth:`__getitem__`, + :meth:`__setitem__` and :meth:`__iter__`. Notes ----- For type testing, ``issubclass(CubeAttrsDict, Mapping)`` is ``True``, but ``issubclass(CubeAttrsDict, dict)`` is ``False``. - Properties 'locals' and 'globals' are the two sets of cube attributes. These are - both :class:`~iris.common.mixin.LimitedAttributeDict` . The CubeAttrsDict object - contains *no* additional state of its own, but simply acts as a view on these two. - - When reading (__getitem__, pop, popitem, keys, values etc), it contains all the - keys + values of both 'locals' and 'globals'. When a key occurs in *both* 'locals' - and 'globals', the result is the local value. - See :meth:`~iris.cube.CubeAttrsDict.__getitem__` . - - When writing (__setitem__, setdefault, update, etc) to a key already present, the - existing entry in either 'locals' or 'globals' is updated. If both are present, - 'locals' is updated. See :meth:`~iris.cube.CubeAttrsDict.__setitem__` . - - When writing to a new key, this generally updates 'locals'. However, certain - specific names would never normally be 'data' attributes, and these are created as - 'globals' instead. These are listed in Appendix A of the - `CF Conventions: `_ . - At present, these are : 'conventions', 'featureType', 'history', 'title'. - Examples -------- @@ -869,12 +852,12 @@ def __init__( """ Create a cube attributes dictionary. - Unlike the attributes of other :class:`~iris.common.mixin.CFVariableMixin` - subclasses, this is a "split" dictionary - i.e. it contains separate global + - local settings. - - We support initialisation from a generic dictionary (using default global/local - name identification rules), or from specific local and global ones. + We support initialisation from a single generic mapping input, using the default + global/local assignment rules explained at :meth:`__setatrr__`, or from + two separate mappings. Two separate dicts can be passed in the ``locals`` + and ``globals`` args, **or** via a ``combined`` arg which has its own + ``.globals`` and ``.locals`` properties -- so this allows passing an existing + :class:`CubeAttrsDict`, which will be copied. Parameters ---------- @@ -898,15 +881,17 @@ def __init__( >>> CubeAttrsDict({'history': 'data-story', 'comment': 'this-cube'}) CubeAttrsDict(globals={'history': 'data-story'}, locals={'comment': 'this-cube'}) - >>> CubeAttrsDict({'history': 'data-story', 'comment': 'this cube'}).globals - {'history': 'data-story'} - >>> CubeAttrsDict(locals={'history': 'local-history'}) CubeAttrsDict(globals={}, locals={'history': 'local-history'}) >>> CubeAttrsDict(globals={'x': 'global'}, locals={'x': 'local'}) CubeAttrsDict(globals={'x': 'global'}, locals={'x': 'local'}) + >>> x1 = CubeAttrsDict(globals={'x': 1}, locals={'y': 2}) + >>> x2 = CubeAttrsDict(x1) + >>> x2 + CubeAttrsDict(globals={'x': 1}, locals={'y': 2}) + """ # First initialise locals + globals, defaulting to empty. self.locals = locals @@ -1000,7 +985,12 @@ def copy(self): # def __iter__(self): - # Ordering: all global keys, then all local ones, but omitting duplicates. + """ + Define the combined iteration order. + + Result is: all global keys, then all local ones, but omitting duplicates. + + """ # NOTE: this means that in the "summary" view, attributes present in both # locals+globals are listed first, amongst the globals, even though they appear # with the *value* from locals. @@ -1032,15 +1022,20 @@ def __setitem__(self, key, value): """ Assign an attribute value. - If there is *no* existing attribute of this name, then one is created, either - local or global, depending on whether the name is in the known set of "normally - global" attribute names defined by CF. See :class:`~iris.cube.CubeAttrsDict`. + This may be assigned in either ``self.locals`` or ``self.globals``, chosen as + follows: + + * If there is an existing setting in either ``.locals`` or ``.globals``, then + that is updated (i.e. overwritten). - If there is an *existing* attribute of this name in either ``self.locals`` or - ``self.globals``, then that one is updated (i.e. overwritten). + * If it is present in *both*, only + ``.locals`` is updated. - If the name is present in *both* locals and globals, then only the local one is - updated. + * If there is *no* existing attribute, it is usually created in ``.locals``. + **However** a handful of "known normally global" cases, as defined by CF, + go into ``.globals`` instead. + At present these are : ('conventions', 'featureType', 'history', 'title'). + See `CF Conventions, Appendix A: `_ . """ # If an attribute of this name is already present, update that @@ -1278,8 +1273,8 @@ def __init__( self.cell_methods = cell_methods - #: A dictionary, with a few restricted keys, for arbitrary - #: Cube metadata. + #: A dictionary for arbitrary Cube metadata. + #: A few keys are restricted - see :class:`CubeAttrsDict`. self.attributes = attributes # Coords From 9efe4f62ea1154951c953dfeb15ca30ada9bd021 Mon Sep 17 00:00:00 2001 From: Patrick Peglar Date: Thu, 20 Jul 2023 14:37:55 +0100 Subject: [PATCH 27/29] Review changes. --- lib/iris/cube.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/iris/cube.py b/lib/iris/cube.py index 7fbc2e46e5..8bb9d7c00e 100644 --- a/lib/iris/cube.py +++ b/lib/iris/cube.py @@ -955,17 +955,17 @@ def __setstate__(self, state): self.locals, self.globals = state # - # Support simple comparison, even when contents are arrays. + # Support comparison -- required because default operation only compares a single + # value at each key. # def __eq__(self, other): - # For equality, require both globals + locals to match + # For equality, require both globals + locals to match exactly. + # NOTE: array content works correctly, since 'locals' and 'globals' are always + # iris.common.mixin.LimitedAttributeDict, which gets this right. other = CubeAttrsDict(other) result = self.locals == other.locals and self.globals == other.globals return result - def __ne__(self, other): - return not self == other - # # Provide a copy method, as for 'dict', but *not* provided by MutableMapping # From 05c4e67709b583ba71cfdaff79069b6b1197d0de Mon Sep 17 00:00:00 2001 From: Patrick Peglar Date: Fri, 4 Nov 2022 10:30:19 +0000 Subject: [PATCH 28/29] Distinguish local+global attributes in netcdf loads. --- lib/iris/fileformats/_nc_load_rules/helpers.py | 2 +- lib/iris/fileformats/netcdf/loader.py | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/iris/fileformats/_nc_load_rules/helpers.py b/lib/iris/fileformats/_nc_load_rules/helpers.py index bbf9c660c5..acbcde103a 100644 --- a/lib/iris/fileformats/_nc_load_rules/helpers.py +++ b/lib/iris/fileformats/_nc_load_rules/helpers.py @@ -433,7 +433,7 @@ def build_cube_metadata(engine): # Set the cube global attributes. for attr_name, attr_value in cf_var.cf_group.global_attributes.items(): try: - cube.attributes[str(attr_name)] = attr_value + cube.attributes.globals[str(attr_name)] = attr_value except ValueError as e: msg = "Skipping global attribute {!r}: {}" warnings.warn(msg.format(attr_name, str(e))) diff --git a/lib/iris/fileformats/netcdf/loader.py b/lib/iris/fileformats/netcdf/loader.py index 20d255ea44..dfdcd990dc 100644 --- a/lib/iris/fileformats/netcdf/loader.py +++ b/lib/iris/fileformats/netcdf/loader.py @@ -159,8 +159,13 @@ def attribute_predicate(item): return item[0] not in _CF_ATTRS tmpvar = filter(attribute_predicate, cf_var.cf_attrs_unused()) + attrs_dict = iris_object.attributes + if hasattr(attrs_dict, "locals"): + # Treat cube attributes (i.e. a CubeAttrsDict) as a special case. + # These attrs are "local" (i.e. on the variable), so record them as such. + attrs_dict = attrs_dict.locals for attr_name, attr_value in tmpvar: - _set_attributes(iris_object.attributes, attr_name, attr_value) + _set_attributes(attrs_dict, attr_name, attr_value) def _get_actual_dtype(cf_var): From 2c60b0275ccbed37cd839f527616662257568a99 Mon Sep 17 00:00:00 2001 From: Patrick Peglar Date: Fri, 4 Nov 2022 12:17:22 +0000 Subject: [PATCH 29/29] Small test fixes. --- lib/iris/fileformats/_nc_load_rules/helpers.py | 2 +- .../nc_load_rules/helpers/test_build_cube_metadata.py | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/iris/fileformats/_nc_load_rules/helpers.py b/lib/iris/fileformats/_nc_load_rules/helpers.py index acbcde103a..f30da59e87 100644 --- a/lib/iris/fileformats/_nc_load_rules/helpers.py +++ b/lib/iris/fileformats/_nc_load_rules/helpers.py @@ -435,7 +435,7 @@ def build_cube_metadata(engine): try: cube.attributes.globals[str(attr_name)] = attr_value except ValueError as e: - msg = "Skipping global attribute {!r}: {}" + msg = "Skipping disallowed global attribute {!r}: {}" warnings.warn(msg.format(attr_name, str(e))) diff --git a/lib/iris/tests/unit/fileformats/nc_load_rules/helpers/test_build_cube_metadata.py b/lib/iris/tests/unit/fileformats/nc_load_rules/helpers/test_build_cube_metadata.py index a13fa6cca0..dfe0379a16 100644 --- a/lib/iris/tests/unit/fileformats/nc_load_rules/helpers/test_build_cube_metadata.py +++ b/lib/iris/tests/unit/fileformats/nc_load_rules/helpers/test_build_cube_metadata.py @@ -42,7 +42,7 @@ def _make_engine(global_attributes=None, standard_name=None, long_name=None): return engine -class TestInvalidGlobalAttributes(tests.IrisTest): +class TestGlobalAttributes(tests.IrisTest): def test_valid(self): global_attributes = { "Conventions": "CF-1.5", @@ -51,7 +51,7 @@ def test_valid(self): engine = _make_engine(global_attributes) build_cube_metadata(engine) expected = global_attributes - self.assertEqual(engine.cube.attributes, expected) + self.assertEqual(engine.cube.attributes.globals, expected) def test_invalid(self): global_attributes = { @@ -65,13 +65,14 @@ def test_invalid(self): # Check for a warning. self.assertEqual(warn.call_count, 1) self.assertIn( - "Skipping global attribute 'calendar'", warn.call_args[0][0] + "Skipping disallowed global attribute 'calendar'", + warn.call_args[0][0], ) # Check resulting attributes. The invalid entry 'calendar' # should be filtered out. global_attributes.pop("calendar") expected = global_attributes - self.assertEqual(engine.cube.attributes, expected) + self.assertEqual(engine.cube.attributes.globals, expected) class TestCubeName(tests.IrisTest):