diff --git a/benchmarks/benchmarks/unit_style/ugrid.py b/benchmarks/benchmarks/unit_style/ugrid.py index fad316af17..e2f235eb28 100644 --- a/benchmarks/benchmarks/unit_style/ugrid.py +++ b/benchmarks/benchmarks/unit_style/ugrid.py @@ -93,9 +93,8 @@ def setup(self, n_faces, lazy=False): ) def get_coords_and_axes(location): - search_kwargs = {f"include_{location}s": True} return [ - (source_mesh.coord(axis=axis, **search_kwargs), axis) + (source_mesh.coord(axis=axis, location=location), axis) for axis in ("x", "y") ] @@ -114,7 +113,7 @@ def get_coords_and_axes(location): self.node_x = self.object.node_coords.node_x # Kwargs for reuse in search and remove methods. self.connectivities_kwarg = dict(cf_role="edge_node_connectivity") - self.coords_kwarg = dict(include_faces=True) + self.coords_kwarg = dict(location="face") # TODO: an opportunity for speeding up runtime if needed, since # eq_object is not needed for all benchmarks. Just don't generate it diff --git a/docs/src/whatsnew/latest.rst b/docs/src/whatsnew/latest.rst index e87a84a777..c2bec897e1 100644 --- a/docs/src/whatsnew/latest.rst +++ b/docs/src/whatsnew/latest.rst @@ -82,6 +82,12 @@ This document explains the changes made to Iris for this release more flexible parent class (:class:`~iris.experimental.ugrid.mesh.Mesh`). (:issue:`6052` :pull:`6056`) +#. `@stephenworsley`_ replaced the ``include_nodes``, ``include_edges`` and + ``include_faces`` arguments with a single ``location`` argument in the + :class:`~iris.experimental.ugrid.Mesh` methods + :meth:`~iris.experimental.ugrid.Mesh.coord`, :meth:`~iris.experimental.ugrid.Mesh.coords` + and :meth:`~iris.experimental.ugrid.Mesh.remove_coords`. (:pull:`6055`) + 🚀 Performance Enhancements =========================== diff --git a/lib/iris/experimental/ugrid/mesh.py b/lib/iris/experimental/ugrid/mesh.py index 1d63712280..c90b67e1d6 100644 --- a/lib/iris/experimental/ugrid/mesh.py +++ b/lib/iris/experimental/ugrid/mesh.py @@ -1042,8 +1042,7 @@ def line(text, i_indent=0): main_conn_string = main_conn.summary(shorten=True, linewidth=0) line(f"{main_conn_name}: {main_conn_string}", 2) # Print coords - include_key = f"include_{element}s" - coords = self.coords(**{include_key: True}) + coords = self.coords(location=element) if coords: line(f"{element} coordinates", 2) for coord in coords: @@ -1527,9 +1526,7 @@ def coord( var_name=None, attributes=None, axis=None, - include_nodes=None, - include_edges=None, - include_faces=None, + location=None, ): """Return a single :class:`~iris.coords.AuxCoord` coordinate. @@ -1577,12 +1574,8 @@ def coord( The desired coordinate axis, see :func:`~iris.util.guess_coord_axis`. If ``None``, does not check for ``axis``. Accepts the values ``X``, ``Y``, ``Z`` and ``T`` (case-insensitive). - include_node : bool, optional - Include all ``node`` coordinates in the list of objects to be matched. - include_edge : bool, optional - Include all ``edge`` coordinates in the list of objects to be matched. - include_face : bool, optional - Include all ``face`` coordinates in the list of objects to be matched. + location : str, optional + The desired location. Accepts the values ``node``, ``edge`` or ``face``. Returns ------- @@ -1598,9 +1591,7 @@ def coord( var_name=var_name, attributes=attributes, axis=axis, - include_nodes=include_nodes, - include_edges=include_edges, - include_faces=include_faces, + location=location, ) return list(result.values())[0] @@ -1612,9 +1603,7 @@ def coords( var_name=None, attributes=None, axis=None, - include_nodes=None, - include_edges=None, - include_faces=None, + location=None, ): """Return all :class:`~iris.coords.AuxCoord` coordinates from the :class:`MeshXY`. @@ -1657,12 +1646,8 @@ def coords( The desired coordinate axis, see :func:`~iris.util.guess_coord_axis`. If ``None``, does not check for ``axis``. Accepts the values ``X``, ``Y``, ``Z`` and ``T`` (case-insensitive). - include_node : bool, optional - Include all ``node`` coordinates in the list of objects to be matched. - include_edge : bool, optional - Include all ``edge`` coordinates in the list of objects to be matched. - include_face : bool, optional - Include all ``face`` coordinates in the list of objects to be matched. + location : str, optional + The desired location. Accepts the values ``node``, ``edge`` or ``face``. Returns ------- @@ -1678,9 +1663,7 @@ def coords( var_name=var_name, attributes=attributes, axis=axis, - include_nodes=include_nodes, - include_edges=include_edges, - include_faces=include_faces, + location=location, ) return list(result.values()) @@ -1778,9 +1761,7 @@ def remove_coords( var_name=None, attributes=None, axis=None, - include_nodes=None, - include_edges=None, - include_faces=None, + location=None, ): """Remove one or more :class:`~iris.coords.AuxCoord` from the :class:`MeshXY`. @@ -1819,15 +1800,8 @@ def remove_coords( The desired coordinate axis, see :func:`~iris.util.guess_coord_axis`. If ``None``, does not check for ``axis``. Accepts the values ``X``, ``Y``, ``Z`` and ``T`` (case-insensitive). - include_node : bool, optional - Include all ``node`` coordinates in the list of objects to be matched - for potential removal. - include_edge : bool, optional - Include all ``edge`` coordinates in the list of objects to be matched - for potential removal. - include_face : bool, optional - Include all ``face`` coordinates in the list of objects to be matched - for potential removal. + location : str, optional + The desired location. Accepts the values ``node``, ``edge`` or ``face``. Returns ------- @@ -1836,22 +1810,17 @@ def remove_coords( the :class:`MeshXY` that matched the given criteria. """ - # Filter out absent arguments - only expecting face coords sometimes, - # same will be true of volumes in future. - kwargs = { - "item": item, - "standard_name": standard_name, - "long_name": long_name, - "var_name": var_name, - "attributes": attributes, - "axis": axis, - "include_nodes": include_nodes, - "include_edges": include_edges, - "include_faces": include_faces, - } - kwargs = {k: v for k, v in kwargs.items() if v} + result = self._coord_manager.remove( + item=item, + standard_name=standard_name, + long_name=long_name, + var_name=var_name, + attributes=attributes, + axis=axis, + location=location, + ) - return self._coord_manager.remove(**kwargs) + return result def xml_element(self, doc): """Create the :class:`xml.dom.minidom.Element` that describes this :class:`MeshXY`. @@ -2243,21 +2212,21 @@ def filters( var_name=None, attributes=None, axis=None, - include_nodes=None, - include_edges=None, - include_faces=None, + location=None, ): # TBD: support coord_systems? - # Preserve original argument before modifying. - face_requested = include_faces - - # Rationalise the tri-state behaviour. - args = [include_nodes, include_edges, include_faces] - state = not any(set(filter(lambda arg: arg is not None, args))) - include_nodes, include_edges, include_faces = map( - lambda arg: arg if arg is not None else state, args - ) + # Determine locations to include. + if location is not None: + if location not in ["node", "edge", "face"]: + raise ValueError( + f"Expected location to be one of `node`, `edge` or `face`, got `{location}`" + ) + include_nodes = location == "node" + include_edges = location == "edge" + include_faces = location == "face" + else: + include_nodes = include_edges = include_faces = True def populated_coords(coords_tuple): return list(filter(None, list(coords_tuple))) @@ -2270,7 +2239,7 @@ def populated_coords(coords_tuple): if hasattr(self, "face_coords"): if include_faces: members += populated_coords(self.face_coords) - elif face_requested: + elif location == "face": dmsg = "Ignoring request to filter non-existent 'face_coords'" logger.debug(dmsg, extra=dict(cls=self.__class__.__name__)) @@ -2297,8 +2266,7 @@ def remove( var_name=None, attributes=None, axis=None, - include_nodes=None, - include_edges=None, + location=None, ): return self._remove( item=item, @@ -2307,8 +2275,7 @@ def remove( var_name=var_name, attributes=attributes, axis=axis, - include_nodes=include_nodes, - include_edges=include_edges, + location=location, ) @@ -2383,9 +2350,7 @@ def remove( var_name=None, attributes=None, axis=None, - include_nodes=None, - include_edges=None, - include_faces=None, + location=None, ): return self._remove( item=item, @@ -2394,9 +2359,7 @@ def remove( var_name=var_name, attributes=attributes, axis=axis, - include_nodes=include_nodes, - include_edges=include_edges, - include_faces=include_faces, + location=location, ) @@ -2771,14 +2734,13 @@ def __init__( raise ValueError(msg) # Get the 'coord identity' metadata from the relevant node-coordinate. - node_coord = self.mesh.coord(include_nodes=True, axis=self.axis) + node_coord = self.mesh.coord(location="node", axis=self.axis) node_metadict = node_coord.metadata._asdict() # Use node metadata, unless location is face/edge. use_metadict = node_metadict.copy() if location != "node": # Location is either "edge" or "face" - get the relevant coord. - kwargs = {f"include_{location}s": True, "axis": axis} - location_coord = self.mesh.coord(**kwargs) + location_coord = self.mesh.coord(location=location, axis=axis) # Take the MeshCoord metadata from the 'location' coord. use_metadict = location_coord.metadata._asdict() @@ -2860,16 +2822,12 @@ def coord_system(self): """ # This matches where the coord metadata is drawn from. # See : https://github.com/SciTools/iris/issues/4860 - select_kwargs = { - f"include_{self.location}s": True, - "axis": self.axis, - } try: # NOTE: at present, a MeshCoord *always* references the relevant location # coordinate in the mesh, from which its points are taken. # However this might change in future .. # see : https://github.com/SciTools/iris/discussions/4438#bounds-no-points - location_coord = self.mesh.coord(**select_kwargs) + location_coord = self.mesh.coord(location=self.location, axis=self.axis) coord_system = location_coord.coord_system except CoordinateNotFoundError: # No such coord : possible in UGRID, but probably not Iris (at present). @@ -3078,17 +3036,14 @@ def _construct_access_arrays(self): """ mesh, location, axis = self.mesh, self.location, self.axis - node_coord = self.mesh.coord(include_nodes=True, axis=axis) + node_coord = mesh.coord(location="node", axis=axis) if location == "node": points_coord = node_coord bounds_connectivity = None - elif location == "edge": - points_coord = self.mesh.coord(include_edges=True, axis=axis) - bounds_connectivity = mesh.edge_node_connectivity - elif location == "face": - points_coord = self.mesh.coord(include_faces=True, axis=axis) - bounds_connectivity = mesh.face_node_connectivity + else: + points_coord = mesh.coord(location=location, axis=axis) + bounds_connectivity = getattr(mesh, f"{location}_node_connectivity") # The points output is the points of the relevant element-type coord. points = points_coord.core_points() diff --git a/lib/iris/fileformats/netcdf/saver.py b/lib/iris/fileformats/netcdf/saver.py index d8fdf64067..179adaf9cd 100644 --- a/lib/iris/fileformats/netcdf/saver.py +++ b/lib/iris/fileformats/netcdf/saver.py @@ -1204,7 +1204,7 @@ def record_dimension(names_list, dim_name, length, matching_coords=None): if location == "node": # For nodes, identify the dim with a coordinate variable. # Selecting the X-axis one for definiteness. - dim_coords = mesh.coords(include_nodes=True, axis="x") + dim_coords = mesh.coords(location="node", axis="x") else: # For face/edge, use the relevant "optionally required" # connectivity variable. diff --git a/lib/iris/tests/integration/experimental/test_meshcoord_coordsys.py b/lib/iris/tests/integration/experimental/test_meshcoord_coordsys.py index 353d56004f..d9ec782108 100644 --- a/lib/iris/tests/integration/experimental/test_meshcoord_coordsys.py +++ b/lib/iris/tests/integration/experimental/test_meshcoord_coordsys.py @@ -97,7 +97,7 @@ def test_assigned_mesh_cs(tmp_path): make_file(nc_path) with PARSE_UGRID_ON_LOAD.context(): cube = iris.load_cube(nc_path, "node_data") - nodeco_x = cube.mesh.coord(include_nodes=True, axis="x") + nodeco_x = cube.mesh.coord(location="node", axis="x") meshco_x, meshco_y = [cube.coord(axis=ax) for ax in ("x", "y")] assert nodeco_x.coord_system is None assert meshco_x.coord_system is None @@ -118,7 +118,7 @@ def test_meshcoord_coordsys_copy(tmp_path): make_file(nc_path) with PARSE_UGRID_ON_LOAD.context(): cube = iris.load_cube(nc_path, "node_data") - node_coord = cube.mesh.coord(include_nodes=True, axis="x") + node_coord = cube.mesh.coord(location="node", axis="x") assigned_cs = GeogCS(1.0) node_coord.coord_system = assigned_cs mesh_coord = cube.coord(axis="x") diff --git a/lib/iris/tests/unit/experimental/ugrid/mesh/test_Mesh.py b/lib/iris/tests/unit/experimental/ugrid/mesh/test_Mesh.py index 39e0aab4e3..a47d093af0 100644 --- a/lib/iris/tests/unit/experimental/ugrid/mesh/test_Mesh.py +++ b/lib/iris/tests/unit/experimental/ugrid/mesh/test_Mesh.py @@ -226,8 +226,11 @@ def test_coord(self): # See MeshXY.coords tests for thorough coverage of cases. func = self.mesh.coord exception = CoordinateNotFoundError - self.assertRaisesRegex(exception, ".*but found 2", func, include_nodes=True) + self.assertRaisesRegex(exception, ".*but found 2", func, location="node") self.assertRaisesRegex(exception, ".*but found none", func, axis="t") + self.assertRaisesRegex( + ValueError, "Expected location.*got `foo`", func, location="foo" + ) def test_coords(self): # General results. Method intended for inheritance. @@ -238,6 +241,7 @@ def test_coords(self): {"long_name": "long_name"}, {"var_name": "node_lon"}, {"attributes": {"test": 1}}, + {"location": "node"}, ) fake_coord = AuxCoord([0]) @@ -248,6 +252,7 @@ def test_coords(self): {"long_name": "foo"}, {"var_name": "foo"}, {"attributes": {"test": 2}}, + {"location": "edge"}, ) func = self.mesh.coords @@ -256,6 +261,10 @@ def test_coords(self): for kwargs in negative_kwargs: self.assertNotIn(self.NODE_LON, func(**kwargs)) + self.assertRaisesRegex( + ValueError, "Expected location.*got.*foo", self.mesh.coords, location="foo" + ) + def test_coords_elements(self): # topology_dimension-specific results. Method intended to be overridden. all_expected = { @@ -268,19 +277,9 @@ def test_coords_elements(self): kwargs_expected = ( ({"axis": "x"}, ["node_x", "edge_x"]), ({"axis": "y"}, ["node_y", "edge_y"]), - ({"include_nodes": True}, ["node_x", "node_y"]), - ({"include_edges": True}, ["edge_x", "edge_y"]), - ({"include_nodes": False}, ["edge_x", "edge_y"]), - ({"include_edges": False}, ["node_x", "node_y"]), - ( - {"include_nodes": True, "include_edges": True}, - ["node_x", "node_y", "edge_x", "edge_y"], - ), - ({"include_nodes": False, "include_edges": False}, []), - ( - {"include_nodes": False, "include_edges": True}, - ["edge_x", "edge_y"], - ), + ({"location": "node"}, ["node_x", "node_y"]), + ({"location": "edge"}, ["edge_x", "edge_y"]), + ({"location": "face"}, ["face_x", "face_y"]), ) func = self.mesh.coords @@ -290,7 +289,7 @@ def test_coords_elements(self): log_regex = r".*filter non-existent.*" with self.assertLogs(logger, level="DEBUG", msg_regex=log_regex): - self.assertEqual([], func(include_faces=True)) + self.assertEqual([], func(location="face")) def test_edge_dimension(self): self.assertEqual(self.kwargs["edge_dimension"], self.mesh.edge_dimension) @@ -571,32 +570,8 @@ def test_coords_elements(self): kwargs_expected = ( ({"axis": "x"}, ["node_x", "edge_x", "face_x"]), ({"axis": "y"}, ["node_y", "edge_y", "face_y"]), - ({"include_nodes": True}, ["node_x", "node_y"]), - ({"include_edges": True}, ["edge_x", "edge_y"]), - ( - {"include_nodes": False}, - ["edge_x", "edge_y", "face_x", "face_y"], - ), - ( - {"include_edges": False}, - ["node_x", "node_y", "face_x", "face_y"], - ), - ( - {"include_faces": False}, - ["node_x", "node_y", "edge_x", "edge_y"], - ), - ( - {"include_faces": True, "include_edges": True}, - ["edge_x", "edge_y", "face_x", "face_y"], - ), - ( - {"include_faces": False, "include_edges": False}, - ["node_x", "node_y"], - ), - ( - {"include_faces": False, "include_edges": True}, - ["edge_x", "edge_y"], - ), + ({"location": "node"}, ["node_x", "node_y"]), + ({"location": "edge"}, ["edge_x", "edge_y"]), ) func = self.mesh.coords @@ -1196,7 +1171,7 @@ def test_remove_coords(self): super().test_remove_coords() self.mesh.add_coords(face_x=self.FACE_LON) self.assertEqual(self.FACE_LON, self.mesh.face_coords.face_x) - self.mesh.remove_coords(include_faces=True) + self.mesh.remove_coords(location="face") self.assertEqual(None, self.mesh.face_coords.face_x) def test_to_MeshCoord_face(self): diff --git a/lib/iris/tests/unit/experimental/ugrid/mesh/test_MeshCoord.py b/lib/iris/tests/unit/experimental/ugrid/mesh/test_MeshCoord.py index fa6e567757..3f81085fa9 100644 --- a/lib/iris/tests/unit/experimental/ugrid/mesh/test_MeshCoord.py +++ b/lib/iris/tests/unit/experimental/ugrid/mesh/test_MeshCoord.py @@ -52,7 +52,7 @@ def test_derived_properties(self): # underlying mesh coordinate. for axis in MeshXY.AXES: meshcoord = sample_meshcoord(axis=axis) - face_x_coord = meshcoord.mesh.coord(include_faces=True, axis=axis) + face_x_coord = meshcoord.mesh.coord(location="face", axis=axis) for key in face_x_coord.metadata._fields: meshval = getattr(meshcoord, key) # All relevant attributes are derived from the face coord. @@ -390,7 +390,7 @@ def test_alternative_location_and_axis(self): def test_str_no_long_name(self): mesh = self.mesh # Remove the long_name of the node coord in the mesh. - node_coord = mesh.coord(include_nodes=True, axis="x") + node_coord = mesh.coord(location="node", axis="x") node_coord.long_name = None # Make a new meshcoord, based on the modified mesh. meshcoord = sample_meshcoord(mesh=self.mesh) @@ -401,7 +401,7 @@ def test_str_no_long_name(self): def test_str_no_attributes(self): mesh = self.mesh # No attributes on the node coord in the mesh. - node_coord = mesh.coord(include_nodes=True, axis="x") + node_coord = mesh.coord(location="node", axis="x") node_coord.attributes = None # Make a new meshcoord, based on the modified mesh. meshcoord = sample_meshcoord(mesh=self.mesh) @@ -412,7 +412,7 @@ def test_str_no_attributes(self): def test_str_empty_attributes(self): mesh = self.mesh # Empty attributes dict on the node coord in the mesh. - node_coord = mesh.coord(include_nodes=True, axis="x") + node_coord = mesh.coord(location="node", axis="x") node_coord.attributes.clear() # Make a new meshcoord, based on the modified mesh. meshcoord = sample_meshcoord(mesh=self.mesh) @@ -743,8 +743,8 @@ def test_meshcoord_leaves_originals_lazy(self): # Fetch the relevant source objects from the mesh. def fetch_sources_from_mesh(): return ( - mesh.coord(include_nodes=True, axis="x"), - mesh.coord(include_faces=True, axis="x"), + mesh.coord(location="node", axis="x"), + mesh.coord(location="face", axis="x"), mesh.face_node_connectivity, ) @@ -795,12 +795,8 @@ def setup_mesh(self, location, axis): mesh = sample_mesh() # Modify the metadata of specific coordinates used in this test. - def select_coord(location, axis): - kwargs = {f"include_{location}s": True, "axis": axis} - return mesh.coord(**kwargs) - - node_coord = select_coord("node", axis) - location_coord = select_coord(location, axis) + node_coord = mesh.coord(axis=axis, location="node") + location_coord = mesh.coord(axis=axis, location=location) for i_place, coord in enumerate((node_coord, location_coord)): coord.standard_name = "longitude" if axis == "x" else "latitude" coord.units = "degrees"