Mesh tests

lib/iris/experimental/ugrid.py

@@ -903,6 +903,18 @@ def __init__(
         edge_dimension=None,
         face_dimension=None,
     ):
+        """
+        .. note::
+
+            :attr:`node_dimension`, :attr:`edge_dimension` and
+            :attr:`face_dimension` are stored to help round-tripping of UGRID
+            files. As such their presence in :class:`Mesh` is not a direct
+            mirror of that written in the UGRID specification, where
+            :attr:`node_dimension` is not mentioned, while
+            :attr:`edge_dimension` is only present for
+            :attr:`topology_dimension` ``>=2``.
+
+        """
         # TODO: support volumes.
         # TODO: support (coord, "z")
 
@@ -1130,7 +1142,16 @@ def face_dimension(self):
 
     @face_dimension.setter
     def face_dimension(self, name):
-        if not name or not isinstance(name, str):
+        if self.topology_dimension < 2:
+            face_dimension = None
+            if name:
+                # Tell the user it is not being set if they expected otherwise.
+                message = (
+                    "Not setting face_dimension (inappropriate for "
+                    f"topology_dimension={self.topology_dimension} ."
+                )
+                logger.debug(message)
+        elif not name or not isinstance(name, str):
             face_dimension = f"Mesh{self.topology_dimension}d_face"
         else:
             face_dimension = name
@@ -1199,14 +1220,19 @@ def add_coords(
         face_x=None,
         face_y=None,
     ):
-        self._coord_manager.add(
-            node_x=node_x,
-            node_y=node_y,
-            edge_x=edge_x,
-            edge_y=edge_y,
-            face_x=face_x,
-            face_y=face_y,
-        )
+        # Filter out absent arguments - only expecting face coords sometimes,
+        # same will be true of volumes in future.
+        kwargs = {
+            "node_x": node_x,
+            "node_y": node_y,
+            "edge_x": edge_x,
+            "edge_y": edge_y,
+            "face_x": face_x,
+            "face_y": face_y,
+        }
+        kwargs = {k: v for k, v in kwargs.items() if v}
+
+        self._coord_manager.add(**kwargs)
 
     def add_connectivities(self, *connectivities):
         self._connectivity_manager.add(*connectivities)
@@ -1291,9 +1317,9 @@ def coords(
         var_name=None,
         attributes=None,
         axis=None,
-        node=False,
-        edge=False,
-        face=False,
+        node=None,
+        edge=None,
+        face=None,
     ):
         return self._coord_manager.filters(
             item=item,
@@ -1343,17 +1369,22 @@ def remove_coords(
         edge=None,
         face=None,
     ):
-        return self._coord_manager.remove(
-            item=item,
-            standard_name=standard_name,
-            long_name=long_name,
-            var_name=var_name,
-            attributes=attributes,
-            axis=axis,
-            node=node,
-            edge=edge,
-            face=face,
-        )
+        # 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,
+            "node": node,
+            "edge": edge,
+            "face": face,
+        }
+        kwargs = {k: v for k, v in kwargs.items() if v}
+
+        return self._coord_manager.remove(**kwargs)
 
     def xml_element(self):
         # TBD
@@ -1657,12 +1688,16 @@ def filters(
     ):
         # TBD: support coord_systems?
 
-        # rationalise the tri-state behaviour
+        face_requested = face is True
         args = [node, edge, face]
-        state = not any(set(filter(lambda arg: arg is not None, args)))
-        node, edge, face = map(
-            lambda arg: arg if arg is not None else state, args
-        )
+        true_count = len([arg for arg in args if arg])
+        if true_count > 1:
+            # Standard filter behaviour is 'AND', and coord locations are
+            # mutually exclusive, so multiple True cannot return any results.
+            node = edge = face = False
+        elif true_count == 0:
+            # Treat None as True in this case.
+            node, edge, face = [True if arg is None else arg for arg in args]
 
         def populated_coords(coords_tuple):
             return list(filter(None, list(coords_tuple)))
@@ -1675,7 +1710,7 @@ def populated_coords(coords_tuple):
         if hasattr(self, "face_coords"):
             if face:
                 members += populated_coords(self.face_coords)
-        else:
+        elif face_requested:
             dmsg = "Ignoring request to filter non-existent 'face_coords'"
             logger.debug(dmsg, extra=dict(cls=self.__class__.__name__))
 
@@ -1820,9 +1855,7 @@ def __init__(self, *connectivities):
         cf_roles = [c.cf_role for c in connectivities]
         for requisite in self.REQUIRED:
             if requisite not in cf_roles:
-                message = (
-                    f"{self.__name__} requires a {requisite} Connectivity."
-                )
+                message = f"{type(self).__name__} requires a {requisite} Connectivity."
                 raise ValueError(message)
 
         self.ALL = self.REQUIRED + self.OPTIONAL
@@ -1880,7 +1913,7 @@ def add(self, *connectivities):
         for connectivity in connectivities:
             if not isinstance(connectivity, Connectivity):
                 message = f"Expected Connectivity, got: {type(connectivity)} ."
-                raise ValueError(message)
+                raise TypeError(message)
             cf_role = connectivity.cf_role
             if cf_role not in self.ALL:
                 message = (