Skip to content

Fix misleading/confusing error types throughout uxarray - #1652

Merged
Sevans711 merged 5 commits into
mainfrom
sevans/error-types-cleanup
Aug 10, 2026
Merged

Fix misleading/confusing error types throughout uxarray#1652
Sevans711 merged 5 commits into
mainfrom
sevans/error-types-cleanup

Conversation

@Sevans711

Copy link
Copy Markdown
Collaborator

Closes #1622

Overview

Fixes error types which did correspond closely to the actual error being reported. For example, for an error like "This functionality is not yet supported" the error type should be NotImplementedError, not ValueError. This PR addresses all cases mentioned in #1622, which is an exhaustive list of all such cases throughout uxarray.

Test suite has been updated accordingly for all tests which failed due to expecting old error types.

Slight expansion beyond original scope: this PR also adds an import of uxarray.errors module in uxarray's top-level __init__.py, to make custom error types easier to refer to. Now users can reliably write something like ux.errors.DimensionError, instead of possibly needing to import uxarray.errors specifically and then do uxarray.errors.DimensionError. Questions for reviewers: (A) is this okay to bundle here? (B) Do I also need to add something to docs/api.rst page?

Below copies the list of changes enumerated in issue 1622, and marks them as (done), possibly with minor notes to show how they have been completed here.

  1. (done) Exception → ValueError in grid.bounds.insert_pt_in_latlonbox: "Longitude point out of range"
  2. (done - chose AssertionError) Exception → AssertionError or ValueError(?) in grid.bounds.insert_pt_in_latlonbox: "Logic error in longitude box width calculation"
  3. (done) Exception → AttributeError in property Grid.inverse_indices: "Grid is not a subset, therefore no inverse face indices exist"
  4. (done) ValueError → TypeError in core.aggregation._node_to_face_aggregation and core.aggregation._node_to_edge_aggregation: (error currently has no message, but is raised to indicate: not isinstance(uxda.data, (np.ndarray, da.Array)).)
  5. (done) ValueError → TypeError in UxDataArray.zonal_anomaly: "Invalid value for 'lat'. Must be a tuple (start, end, step) or array-like band edges."
  6. (done) ValueError → TypeError in Grid.from_face_vertices: "Input must be either a list, tuple, or np.ndarray"
  7. (done) ValueError → NotImplementedError in Grid.boundary_node_indices: (error currently has no message, but seems like it is supposed to indicate that construction of boundary_node_indices is not yet implemented.)
  8. (done) TypeError → ValueError in grid.neighbors.KDTree._build_from_nodes: f"Unknown coordinate_system, {self.coordinate_system}, use either 'cartesian' or 'spherical'".
  9. (done) TypeError → ValueError in grid.neighbors.KDTree._current_tree and grid.neighbors.BallTree._current_tree: f"Unknown coordinates location, {self._coordinates}, use either 'nodes', 'face centers', or 'edge centers'"
  10. (done) ValueError → NotImplementedError in grid.slice._slice_node_indices, grid.slice._slice_node_indices, and grid.slice._slice_face_indices: "Exclusive slicing is not yet supported."
  11. (done) ValueError → TypeError in grid.utils.make_setter: f"{key} must be an xr.DataArray" (raised if not isinstance(value, xr.DataArray)).
  12. (done) ValueError → NotImplementedError in io._esmf._read_esmf: "Reading in ESMF grids with Cartesian coordinates not yet supported"
  13. (done) ValueError → NotImplementedError in io._icon._read_icon: "Conversion of the ICON Dual mesh is not yet supported."
  14. (done) Exception → NotImplementedError in io._scrip._to_ugrid: "Structured scrip files are not yet supported"
  15. (done) ValueError → AttributeError in remap.yac._get_lon_lat: f"Grid does not provide {lon_attr}/{lat_attr} required for YAC remapping."
  16. (done) RuntimeError → GridInvalidError, in UxDataArray.get_dual, UxDataset.get_dual, and Grid.get_dual: "Duplicate nodes found, cannot construct dual"
  17. (done) RuntimeError → DimensionError, in Grid.from_face_vertices: f"Invalid Input Dimension: {face_vertices.ndim}. Expected dimension should be 3: [n_face, n_node, two/three] or 2 when only one face is passed in."
  18. (done) RuntimeError → GridInvalidError, in Grid.validate: "Mesh validation failed."
  19. (done) AssertionError → DimensionError, in grid.neighbors._prepare_xy_for_query: "The dimension of each coordinate pair must be two (lon, lat). Did you attempt to query using Cartesian (x, y, z) coordinates?", and "The dimension of each coordinate pair must be two (lon, lat).)"
  20. (done) AssertionError → DimensionError, in grid.neighbors._prepare_xyz_for_query: (similar messages as above)
  21. (done) RuntimeError → GridInvalidError, in io.utils._parse_grid_type: "Failed to parse uxgrid information from xarray.Dataset."

PR Checklist

General

  • An issue is created and linked
  • Added appropriate labels (if your uxarray repo permissions allow it)
  • Filled out Overview and Expected Usage (if applicable) sections

Testing & Benchmarking

  • Adequate tests are created if there is new functionality
  • Tests are not too basic (such as simply calling a function and nothing else)
  • [N/A] Tests cover all major paths in your new functions
  • [N/A] If this PR could affect performance, ran ASV benchmarks and confirmed they show expected behavior (add a new benchmark if necessary)

This means users can reliably refer to, e.g., ux.errors.DimensionError, instead of possibly needing to import uxarray.errors specifically and then do uxarray.errors.DimensionError.
@Sevans711 Sevans711 added improvement Improvements on existing features or infrastructure developer experience Makes the codebase easier to read, debug, maintain, or extend. labels Aug 6, 2026

@cmdupuis3 cmdupuis3 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked through all the file changes, nothing seems amiss to me. I also force-tripped a few of these errors and they did what they were supposed to.

@dylannelson dylannelson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!
Made one small comment I think is mostly optional. Just a single error seemed a bit vague without a message. Feel free to merge without resolving or marking it as resolved if you think it doesn't need an error message.
I think a lot of the 'Value' -> 'Type' error switches make sense here. Also switching to the newly implemented errors is a nice touch. Thank you!

Comment thread uxarray/grid/grid.py
@Sevans711
Sevans711 merged commit 5ef66d8 into main Aug 10, 2026
14 checks passed
@Sevans711
Sevans711 deleted the sevans/error-types-cleanup branch August 10, 2026 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

developer experience Makes the codebase easier to read, debug, maintain, or extend. improvement Improvements on existing features or infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update uxarray error types for cases with misleading/confusing types

3 participants