-
Notifications
You must be signed in to change notification settings - Fork 313
Update MeshXY's .filters method to work with coordinates that do not have a standard name and allow the mesh to be saved #7185
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
5be6d48
5dd8aca
73afb39
a5983c3
0f6d66c
89881d8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1711,12 +1711,12 @@ def equal(self, other, lenient=None): | |
|
|
||
| def metadata_filter( | ||
| instances, | ||
| item=None, | ||
| standard_name=None, | ||
| long_name=None, | ||
| var_name=None, | ||
| attributes=None, | ||
| axis=None, | ||
| item: str | None | Any = None, | ||
| standard_name: str | None = None, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You know more than I do in this space. Is
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's mostly a personal choice and it's also best practice: https://typing.python.org/en/latest/reference/best_practices.html#shorthand-syntax . It's easier to write and doesn't need to import something. There has been some discussion I've also seen discussions where they could have a different semantic meanings: https://stackoverflow.com/a/73591412 (not that I agree with them) |
||
| long_name: str | None = None, | ||
| var_name: str | None = None, | ||
| attributes: Mapping | None | Any = None, | ||
| axis: str | None = None, | ||
| ): | ||
| """Filter a collection of objects by their metadata to fit the given metadata criteria. | ||
|
|
||
|
|
@@ -1746,9 +1746,9 @@ def metadata_filter( | |
| var_name : optional | ||
| The NetCDF variable name of the desired object. If ``None``, does | ||
| not check for ``var_name``. | ||
| attributes : dict, optional | ||
| A dictionary of attributes desired on the object. If ``None``, | ||
| does not check for ``attributes``. | ||
| attributes : Mapping, optional, Any | ||
| A mapping of attributes desired on the object. If ``None``, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My suggestion is less technically pure, but it would be helpful to find a way to mention
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah that's fair. I'll add it as part of the comments. I actually can't remember why I changed it from a |
||
| does not check for ``attributes``. Will error if it is anything else. | ||
| axis : optional | ||
| The desired object's axis, see :func:`~iris.util.guess_coord_axis`. | ||
| If ``None``, does not check for ``axis``. Accepts the values ``X``, | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -19,7 +19,7 @@ | |||||
| from collections.abc import Container | ||||||
| from contextlib import contextmanager | ||||||
| from datetime import datetime | ||||||
| from typing import Iterable, Literal | ||||||
| from typing import Any, Iterable, Literal, Mapping, NamedTuple | ||||||
| import warnings | ||||||
|
|
||||||
| from cf_units import Unit | ||||||
|
|
@@ -67,12 +67,25 @@ | |||||
| "Mesh2DCoords", | ||||||
| ["node_x", "node_y", "edge_x", "edge_y", "face_x", "face_y"], | ||||||
| ) | ||||||
|
|
||||||
|
|
||||||
| #: Namedtuple for ``node`` :class:`~iris.coords.AuxCoord` coordinates. | ||||||
| MeshNodeCoords = namedtuple("MeshNodeCoords", ["node_x", "node_y"]) | ||||||
| class MeshNodeCoords(NamedTuple): | ||||||
| node_x: AuxCoord | None | ||||||
| node_y: AuxCoord | None | ||||||
|
|
||||||
|
|
||||||
| #: Namedtuple for ``edge`` :class:`~iris.coords.AuxCoord` coordinates. | ||||||
| MeshEdgeCoords = namedtuple("MeshEdgeCoords", ["edge_x", "edge_y"]) | ||||||
| class MeshEdgeCoords(NamedTuple): | ||||||
| edge_x: AuxCoord | None | ||||||
| edge_y: AuxCoord | None | ||||||
|
|
||||||
|
|
||||||
| #: Namedtuple for ``face`` :class:`~iris.coords.AuxCoord` coordinates. | ||||||
| MeshFaceCoords = namedtuple("MeshFaceCoords", ["face_x", "face_y"]) | ||||||
| class MeshFaceCoords(NamedTuple): | ||||||
| face_x: AuxCoord | None | ||||||
| face_y: AuxCoord | None | ||||||
|
|
||||||
|
|
||||||
| # | ||||||
| # MeshXY connectivity manager namedtuples. | ||||||
|
|
@@ -1623,13 +1636,13 @@ def coord( | |||||
|
|
||||||
| def coords( | ||||||
| self, | ||||||
| item=None, | ||||||
| standard_name=None, | ||||||
| long_name=None, | ||||||
| var_name=None, | ||||||
| attributes=None, | ||||||
| axis=None, | ||||||
| location=None, | ||||||
| item: str | object | None = None, | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| standard_name: str | None = None, | ||||||
| long_name: str | None = None, | ||||||
| var_name: str | None = None, | ||||||
| attributes: Mapping | None = None, | ||||||
| axis: str | None = None, | ||||||
| location: str | None = None, | ||||||
| ): | ||||||
| """Return all :class:`~iris.coords.AuxCoord` coordinates from the :class:`MeshXY`. | ||||||
|
|
||||||
|
|
@@ -1665,8 +1678,8 @@ def coords( | |||||
| var_name : str, optional | ||||||
| The NetCDF variable name of the desired coordinate. If ``None``, does | ||||||
| not check for ``var_name``. | ||||||
| attributes : dict, optional | ||||||
| A dictionary of attributes desired on the coordinates. If ``None``, | ||||||
| attributes : Mapping, optional | ||||||
| A mapping of attributes desired on the coordinates. If ``None``, | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again, some way to communicate to novice users that |
||||||
| does not check for ``attributes``. | ||||||
| axis : str, optional | ||||||
| The desired coordinate axis, see :func:`~iris.util.guess_coord_axis`. | ||||||
|
|
@@ -2008,7 +2021,7 @@ class _Mesh1DCoordinateManager: | |||||
|
|
||||||
| """ | ||||||
|
|
||||||
| REQUIRED = ( | ||||||
| REQUIRED: tuple[str, ...] = ( | ||||||
| "node_x", | ||||||
| "node_y", | ||||||
| ) | ||||||
|
|
@@ -2144,11 +2157,11 @@ def _node_shape(self): | |||||
| return self._shape(element="node") | ||||||
|
|
||||||
| @property | ||||||
| def _members(self): | ||||||
| def _members(self) -> dict[str, None] | dict[str, AuxCoord]: | ||||||
| return self._members_dict | ||||||
|
|
||||||
| @_members.setter | ||||||
| def _members(self, value): | ||||||
| def _members(self, value: dict[str, AuxCoord]): | ||||||
| self.timestamp.update() | ||||||
| self._members_dict = value | ||||||
|
|
||||||
|
|
@@ -2161,15 +2174,15 @@ def edge_coords(self): | |||||
| return MeshEdgeCoords(edge_x=self.edge_x, edge_y=self.edge_y) | ||||||
|
|
||||||
| @property | ||||||
| def edge_x(self): | ||||||
| def edge_x(self) -> AuxCoord | None: | ||||||
| return self._members["edge_x"] | ||||||
|
|
||||||
| @edge_x.setter | ||||||
| def edge_x(self, coord): | ||||||
| self._setter(element="edge", axis="x", coord=coord, shape=self._edge_shape) | ||||||
|
|
||||||
| @property | ||||||
| def edge_y(self): | ||||||
| def edge_y(self) -> AuxCoord | None: | ||||||
| return self._members["edge_y"] | ||||||
|
|
||||||
| @edge_y.setter | ||||||
|
|
@@ -2181,15 +2194,15 @@ def node_coords(self): | |||||
| return MeshNodeCoords(node_x=self.node_x, node_y=self.node_y) | ||||||
|
|
||||||
| @property | ||||||
| def node_x(self): | ||||||
| def node_x(self) -> None | AuxCoord: | ||||||
| return self._members["node_x"] | ||||||
|
|
||||||
| @node_x.setter | ||||||
| def node_x(self, coord): | ||||||
| self._setter(element="node", axis="x", coord=coord, shape=self._node_shape) | ||||||
|
|
||||||
| @property | ||||||
| def node_y(self): | ||||||
| def node_y(self) -> None | AuxCoord: | ||||||
| return self._members["node_y"] | ||||||
|
|
||||||
| @node_y.setter | ||||||
|
|
@@ -2269,44 +2282,61 @@ def filter(self, **kwargs): | |||||
|
|
||||||
| return result | ||||||
|
|
||||||
| @staticmethod | ||||||
| def _populated_coords(coords_tuple: Iterable[AuxCoord | None]) -> list[AuxCoord]: | ||||||
| return list(filter(None, list(coords_tuple))) | ||||||
|
|
||||||
| def mesh_filters(self, axis: str | None, location: str | None) -> list[AuxCoord]: | ||||||
| def get_node(axis: str | None) -> list[AuxCoord]: | ||||||
| match axis: | ||||||
| case "x": | ||||||
| return self._populated_coords((self.node_x,)) | ||||||
| case "y": | ||||||
| return self._populated_coords((self.node_y,)) | ||||||
| case None: | ||||||
| return self._populated_coords(self.node_coords) | ||||||
| case _: | ||||||
| return [] | ||||||
|
|
||||||
| def get_edge(axis: str | None) -> list[AuxCoord]: | ||||||
| match axis: | ||||||
| case "x": | ||||||
| return self._populated_coords((self.edge_x,)) | ||||||
| case "y": | ||||||
| return self._populated_coords((self.edge_y,)) | ||||||
| case None: | ||||||
| return self._populated_coords(self.edge_coords) | ||||||
| case _: | ||||||
| return [] | ||||||
|
|
||||||
| members: list[AuxCoord] = [] | ||||||
| match location: | ||||||
| case "node": | ||||||
| members += get_node(axis) | ||||||
| case "edge": | ||||||
| members += get_edge(axis) | ||||||
| case None: # No specified locations means include them all | ||||||
| members += get_node(axis) | ||||||
| members += get_edge(axis) | ||||||
| case _: | ||||||
| raise ValueError( | ||||||
| f"Expected location to be one of `node` or `edge`, got `{location}`" | ||||||
| ) | ||||||
|
|
||||||
| return members | ||||||
|
|
||||||
| def filters( | ||||||
| self, | ||||||
| item=None, | ||||||
| standard_name=None, | ||||||
| long_name=None, | ||||||
| var_name=None, | ||||||
| attributes=None, | ||||||
| axis=None, | ||||||
| location=None, | ||||||
| item: str | None | Any = None, | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| standard_name: str | None = None, | ||||||
| long_name: str | None = None, | ||||||
| var_name: str | None = None, | ||||||
| attributes: dict | None = None, | ||||||
| axis: str | None = None, | ||||||
| location: str | None = None, | ||||||
| ): | ||||||
| # TBD: support coord_systems? | ||||||
|
|
||||||
| # 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))) | ||||||
|
|
||||||
| members = [] | ||||||
| if include_nodes: | ||||||
| members += populated_coords(self.node_coords) | ||||||
| if include_edges: | ||||||
| members += populated_coords(self.edge_coords) | ||||||
| if hasattr(self, "face_coords"): | ||||||
| if include_faces: | ||||||
| members += populated_coords(self.face_coords) | ||||||
| elif location == "face": | ||||||
| dmsg = "Ignoring request to filter non-existent 'face_coords'" | ||||||
| logger.debug(dmsg, extra=dict(cls=self.__class__.__name__)) | ||||||
| members = self.mesh_filters(axis, location) | ||||||
|
|
||||||
| result = metadata_filter( | ||||||
| members, | ||||||
|
|
@@ -2315,7 +2345,6 @@ def populated_coords(coords_tuple): | |||||
| long_name=long_name, | ||||||
| var_name=var_name, | ||||||
| attributes=attributes, | ||||||
| axis=axis, | ||||||
| ) | ||||||
|
|
||||||
| # Use the results to filter the _members dict for returning. | ||||||
|
|
@@ -2384,15 +2413,15 @@ def face_coords(self): | |||||
| return MeshFaceCoords(face_x=self.face_x, face_y=self.face_y) | ||||||
|
|
||||||
| @property | ||||||
| def face_x(self): | ||||||
| def face_x(self) -> AuxCoord | None: | ||||||
| return self._members["face_x"] | ||||||
|
|
||||||
| @face_x.setter | ||||||
| def face_x(self, coord): | ||||||
| self._setter(element="face", axis="x", coord=coord, shape=self._face_shape) | ||||||
|
|
||||||
| @property | ||||||
| def face_y(self): | ||||||
| def face_y(self) -> AuxCoord | None: | ||||||
| return self._members["face_y"] | ||||||
|
|
||||||
| @face_y.setter | ||||||
|
|
@@ -2431,6 +2460,33 @@ def remove( | |||||
| location=location, | ||||||
| ) | ||||||
|
|
||||||
| def mesh_filters(self, axis: str | None, location: str | None) -> list[AuxCoord]: | ||||||
| def get_face(axis: str | None) -> list[AuxCoord]: | ||||||
| match axis: | ||||||
| case "x": | ||||||
| return self._populated_coords((self.face_x,)) | ||||||
| case "y": | ||||||
| return self._populated_coords((self.face_y,)) | ||||||
| case None: | ||||||
| return self._populated_coords(self.face_coords) | ||||||
| case _: | ||||||
| return [] | ||||||
|
|
||||||
| if location == "face": | ||||||
| return get_face(axis) | ||||||
| elif location is None: | ||||||
| # super().mesh_filters deals with the node and edge | ||||||
| members = super().mesh_filters(axis, location) | ||||||
| return members + get_face(axis) | ||||||
| else: | ||||||
| try: | ||||||
| return super().mesh_filters(axis, location) | ||||||
| except ValueError: | ||||||
| # Update error message (if any) to include mentioning face | ||||||
| raise ValueError( | ||||||
| f"Expected location to be one of `node`, `edge` or `face`, got `{location}`" | ||||||
| ) | ||||||
|
|
||||||
|
|
||||||
| class _MeshConnectivityManagerBase(ABC): | ||||||
| # Override these in subclasses. | ||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.