From f736c4baa56bbf14a6a90d1e21175ada7c008ac7 Mon Sep 17 00:00:00 2001 From: Patrick Peglar Date: Fri, 4 Nov 2022 10:30:19 +0000 Subject: [PATCH 1/4] 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 5483f9a3b821f849cb4775079a810b648b9c6d02 Mon Sep 17 00:00:00 2001 From: Patrick Peglar Date: Fri, 4 Nov 2022 12:17:22 +0000 Subject: [PATCH 2/4] 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): From 54dad2d6faeb5f71e4d5539e6ccc1c0ce12aafcc Mon Sep 17 00:00:00 2001 From: Patrick Peglar Date: Thu, 20 Jul 2023 15:28:32 +0100 Subject: [PATCH 3/4] Small doctest fix. --- 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 875eb62dc3..1c140e7ccd 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=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=()),)) + CubeMetadata(standard_name='air_temperature', long_name=None, var_name='air_temperature', units=Unit('K'), attributes=CubeAttrsDict(globals={'Conventions': 'CF-1.5'}, locals={'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: @@ -676,7 +676,7 @@ For example, consider the following :class:`~iris.common.metadata.CubeMetadata`, .. doctest:: metadata-combine >>> 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=()),)) + CubeMetadata(standard_name='air_temperature', long_name=None, var_name='air_temperature', units=Unit('K'), attributes=CubeAttrsDict(globals={'Conventions': 'CF-1.5'}, locals={'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, @@ -811,7 +811,7 @@ the ``from_metadata`` class method. For example, given the following .. doctest:: metadata-convert >>> 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=()),)) + CubeMetadata(standard_name='air_temperature', long_name=None, var_name='air_temperature', units=Unit('K'), attributes=CubeAttrsDict(globals={'Conventions': 'CF-1.5'}, locals={'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``, From 522265ec3e90aa7d63f13d0d656430330b3c52aa Mon Sep 17 00:00:00 2001 From: Patrick Peglar Date: Thu, 20 Jul 2023 16:13:24 +0100 Subject: [PATCH 4/4] Fix attribute load-save tests for new behaviour, and old-behaviour equivalence. --- .../integration/test_netcdf__loadsaveattrs.py | 86 +++++++++++++++---- 1 file changed, 71 insertions(+), 15 deletions(-) diff --git a/lib/iris/tests/integration/test_netcdf__loadsaveattrs.py b/lib/iris/tests/integration/test_netcdf__loadsaveattrs.py index fa79787950..a1cad53336 100644 --- a/lib/iris/tests/integration/test_netcdf__loadsaveattrs.py +++ b/lib/iris/tests/integration/test_netcdf__loadsaveattrs.py @@ -25,7 +25,7 @@ import iris import iris.coord_systems -from iris.cube import Cube +from iris.cube import Cube, CubeAttrsDict import iris.fileformats.netcdf import iris.fileformats.netcdf._thread_safe_nc as threadsafe_nc4 @@ -633,8 +633,13 @@ def test_01_userstyle_single_global(self): ) # Default behaviour for a general global user-attribute. # It is attached to all loaded cubes. - assert cube1.attributes == {"myname": "single-value"} - assert cube2.attributes == {"myname": "single-value"} + + expected_dict = {"myname": "single-value"} + for cube in (cube1, cube2): + # #1 : legacy results, for cube.attributes **viewed as a plain dictionary**. + assert dict(cube1.attributes) == expected_dict + # #2 : exact expected result, viewed as newstyle split-attributes + assert cube1.attributes == CubeAttrsDict(globals=expected_dict) def test_02_userstyle_single_local(self): # Default behaviour for a general local user-attribute. @@ -658,10 +663,26 @@ def test_03_userstyle_multiple_different(self): global_value_file2="global_file2", vars_values_file2=vars2, ) - assert cube1.attributes == {"random": "f1v1"} - assert cube2.attributes == {"random": "f1v2"} - assert cube3.attributes == {"random": "x1"} - assert cube4.attributes == {"random": "x2"} + + # (#1) : legacy equivalence : for cube.attributes viewed as a plain 'dict' + assert dict(cube1.attributes) == {"random": "f1v1"} + assert dict(cube2.attributes) == {"random": "f1v2"} + assert dict(cube3.attributes) == {"random": "x1"} + assert dict(cube4.attributes) == {"random": "x2"} + + # (#1) : exact results check, for newstyle "split" cube attrs + assert cube1.attributes == CubeAttrsDict( + globals={"random": "global_file1"}, locals={"random": "f1v1"} + ) + assert cube2.attributes == CubeAttrsDict( + globals={"random": "global_file1"}, locals={"random": "f1v2"} + ) + assert cube3.attributes == CubeAttrsDict( + globals={"random": "global_file2"}, locals={"random": "x1"} + ) + assert cube4.attributes == CubeAttrsDict( + globals={"random": "global_file2"}, locals={"random": "x2"} + ) def test_04_userstyle_multiple_same(self): # Nothing special to note in this case @@ -671,8 +692,14 @@ def test_04_userstyle_multiple_same(self): global_value_file1="global_file1", vars_values_file1={"v1": "same-value", "v2": "same-value"}, ) - assert cube1.attributes == {"random": "same-value"} - assert cube2.attributes == {"random": "same-value"} + for cube in (cube1, cube2): + # (#1): legacy values, for cube.attributes viewed as a single dict + assert dict(cube.attributes) == {"random": "same-value"} + # (#2): exact results, with newstyle "split" cube attrs + assert cube2.attributes == CubeAttrsDict( + globals={"random": "global_file1"}, + locals={"random": "same-value"}, + ) ####################################################### # Tests on "Conventions" attribute. @@ -702,7 +729,13 @@ def test_08_conventions_var_both(self): global_value_file1="global-setting", vars_values_file1="local-setting", ) - assert cube.attributes == {"Conventions": "local-setting"} + # (#1): legacy values, for cube.attributes viewed as a single dict + assert dict(cube.attributes) == {"Conventions": "local-setting"} + # (#2): exact results, with newstyle "split" cube attrs + assert cube.attributes == CubeAttrsDict( + globals={"Conventions": "global-setting"}, + locals={"Conventions": "local-setting"}, + ) ####################################################### # Tests on "global" style attributes @@ -725,7 +758,12 @@ def test_10_globalstyle__local(self, global_attr): attr_name=global_attr, vars_values_file1=attr_content, ) - assert cube.attributes == {global_attr: attr_content} + # (#1): legacy values, for cube.attributes viewed as a single dict + assert dict(cube.attributes) == {global_attr: attr_content} + # (#2): exact results, with newstyle "split" cube attrs + assert cube.attributes == CubeAttrsDict( + locals={global_attr: attr_content} + ) def test_11_globalstyle__both(self, global_attr): attr_global = f"Global-{global_attr}" @@ -736,7 +774,13 @@ def test_11_globalstyle__both(self, global_attr): vars_values_file1=attr_local, ) # promoted local setting "wins" - assert cube.attributes == {global_attr: attr_local} + # (#1): legacy values, for cube.attributes viewed as a single dict + assert dict(cube.attributes) == {global_attr: attr_local} + # (#2): exact results, with newstyle "split" cube attrs + assert cube.attributes == CubeAttrsDict( + globals={global_attr: attr_global}, + locals={global_attr: attr_local}, + ) def test_12_globalstyle__multivar_different(self, global_attr): # Multiple *different* local settings are retained @@ -746,8 +790,12 @@ def test_12_globalstyle__multivar_different(self, global_attr): attr_name=global_attr, vars_values_file1={"v1": attr_1, "v2": attr_2}, ) - assert cube1.attributes == {global_attr: attr_1} - assert cube2.attributes == {global_attr: attr_2} + # (#1): legacy values, for cube.attributes viewed as a single dict + assert dict(cube1.attributes) == {global_attr: attr_1} + assert dict(cube2.attributes) == {global_attr: attr_2} + # (#2): exact results, with newstyle "split" cube attrs + assert cube1.attributes == CubeAttrsDict(locals={global_attr: attr_1}) + assert cube2.attributes == CubeAttrsDict(locals={global_attr: attr_2}) def test_14_globalstyle__multifile_different(self, global_attr): # Different global attributes from multiple files are retained as local ones @@ -812,7 +860,15 @@ def test_16_localstyle(self, local_attr, origin_style): # For some reason, these ones never appear on the cube expected_result = {} - assert cube.attributes == expected_result + if origin_style == "input_local": + expected_result_newstyle = CubeAttrsDict(expected_result) + else: + expected_result_newstyle = CubeAttrsDict(globals=expected_result) + + # (#1): legacy values, for cube.attributes viewed as a single dict + assert dict(cube.attributes) == expected_result + # (#2): exact results, with newstyle "split" cube attrs + assert cube.attributes == expected_result_newstyle class TestSave(MixinAttrsTesting):