Conversation
There was a problem hiding this comment.
Pull request overview
Tightens _parse_grid_type format detection by introducing per-format helper checks and adds targeted tests to ensure supported formats are correctly identified while incomplete signals are rejected.
Changes:
- Added helper predicates (
_is_exodus,_is_scrip,_is_mpas, etc.) and refactored_parse_grid_typeto use them. - Expanded supported-format detection to include additional grid specs (e.g., GEOS-CS, ICON, FESOM2) via stricter checks.
- Added new unit tests covering supported formats, structured grids, and rejection of incomplete format signals.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
uxarray/io/utils.py |
Adds per-format detection helpers and refactors _parse_grid_type to use stricter, format-specific checks. |
test/io/test_utils.py |
Adds focused tests for correct format detection and for rejecting incomplete/ambiguous datasets. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def _parse_grid_type(dataset): | ||
| """Checks input and contents to determine grid type. Supports detection of | ||
| UGrid, SCRIP, Exodus, ESMF, and shape file. |
There was a problem hiding this comment.
The _parse_grid_type docstring is out of sync with the implementation: it mentions only UGrid/SCRIP/Exodus/ESMF/shapefile support, but the function now also detects MPAS, GEOS-CS, ICON, FESOM2, and Structured (and there is no shapefile branch here). It also describes returning only mesh_type: str, while the function returns a 3-tuple (mesh_type, lon_name, lat_name). Please update the docstring accordingly so callers/tests have an accurate contract.
Closes #238 by tightening internal grid format detection and adding focused parser tests.