From 835e1344ac50b39e3eba38b71c3dbb1e35997816 Mon Sep 17 00:00:00 2001 From: Rajeev Jain Date: Wed, 19 Aug 2026 17:09:59 -0500 Subject: [PATCH 1/6] Raise the underlying cause when geospatial data cannot be read --- test/io/test_geopandas.py | 15 +++++++++++++++ uxarray/io/_geopandas.py | 8 ++++++-- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/test/io/test_geopandas.py b/test/io/test_geopandas.py index 9e23391f9..ec5d65edc 100644 --- a/test/io/test_geopandas.py +++ b/test/io/test_geopandas.py @@ -40,3 +40,18 @@ def test_load_xarray_with_from_file(gridpath): nc_filename = gridpath("scrip", "outCSne8", "outCSne8.nc") uxgrid = ux.Grid.from_file(nc_filename, backend="xarray") uxgrid.validate() + + +def test_read_failure_raises(tmp_path): + """A read failure must surface its cause, not an UnboundLocalError. + Regression test for issue #1693.""" + import pytest + + from uxarray.errors import GridInvalidError + from uxarray.io._geopandas import _gpd_read + + not_geospatial = tmp_path / "not_geospatial.shp" + not_geospatial.write_text("this is not a shapefile") + + with pytest.raises(GridInvalidError, match="Could not read"): + _gpd_read(str(not_geospatial)) diff --git a/uxarray/io/_geopandas.py b/uxarray/io/_geopandas.py index a47606ac2..6fa174f16 100644 --- a/uxarray/io/_geopandas.py +++ b/uxarray/io/_geopandas.py @@ -3,6 +3,7 @@ from uxarray.constants import INT_DTYPE, INT_FILL_VALUE, WGS84_CRS from uxarray.conventions import ugrid +from uxarray.errors import GridInvalidError def _read_geodataframe(filepath, driver=None, **kwargs): @@ -68,9 +69,12 @@ def _gpd_read(filepath, driver=None, **kwargs): try: gdf = gpd.read_file(filepath, driver=driver, **kwargs) - gdf = _set_crs(gdf) except Exception as e: - print(f"An error occurred while reading the geospatial data: {e}") + raise GridInvalidError( + f"Could not read geospatial data from {filepath!r}: {e}" + ) from e + + gdf = _set_crs(gdf) max_polygon_nodes = gdf["geometry"].apply(_get_num_nodes).max() From 8b82e63c076ff66de3d7421eaec47e7311a44cca Mon Sep 17 00:00:00 2001 From: Rajeev Jain Date: Thu, 20 Aug 2026 12:23:34 -0500 Subject: [PATCH 2/6] Let geopandas raise its own read errors and pin geopandas>=1.0 The previous handler caught every read failure, printed it, and left gdf unbound, so the next line died with UnboundLocalError instead of the real cause. Wrapping the failure in GridInvalidError hid the backend's own exception type, and no other reader in uxarray/io wraps its backend's errors, so drop the try/except and let gpd.read_file raise. The underlying problem was packaging: geopandas was unpinned and releases before 1.0 do not require pyogrio, so an install could end up with geopandas and no file-IO backend at all, making every read_file call fail with ImportError. The regression test now skips without pyogrio, since it would otherwise pass on that ImportError rather than on an actual parse failure. --- ci/environment.yml | 2 +- pyproject.toml | 2 +- test/io/test_geopandas.py | 16 ++++++++++++---- uxarray/io/_geopandas.py | 9 +-------- 4 files changed, 15 insertions(+), 14 deletions(-) diff --git a/ci/environment.yml b/ci/environment.yml index 44897b982..938470f61 100644 --- a/ci/environment.yml +++ b/ci/environment.yml @@ -32,6 +32,6 @@ dependencies: - shapely - spatialpandas - pooch - - geopandas + - geopandas>=1.0 - xarray - asv diff --git a/pyproject.toml b/pyproject.toml index 0596956c6..0118799a3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -40,7 +40,7 @@ dependencies = [ "scipy", "shapely", "spatialpandas", - "geopandas", + "geopandas>=1.0", # 1.0 is the first release requiring pyogrio, the file-IO backend read_file needs. See #1693. "xarray", "hvplot", "healpix", diff --git a/test/io/test_geopandas.py b/test/io/test_geopandas.py index ec5d65edc..fd98009db 100644 --- a/test/io/test_geopandas.py +++ b/test/io/test_geopandas.py @@ -43,15 +43,23 @@ def test_load_xarray_with_from_file(gridpath): def test_read_failure_raises(tmp_path): - """A read failure must surface its cause, not an UnboundLocalError. - Regression test for issue #1693.""" + """A read failure must surface the backend's own error rather than being + printed and swallowed into an UnboundLocalError. + + Regression test for issue #1693. Requires a geopandas file-IO backend, + otherwise read_file raises ImportError and the test would pass for the + wrong reason. + """ import pytest - from uxarray.errors import GridInvalidError + pytest.importorskip("pyogrio") + from uxarray.io._geopandas import _gpd_read not_geospatial = tmp_path / "not_geospatial.shp" not_geospatial.write_text("this is not a shapefile") - with pytest.raises(GridInvalidError, match="Could not read"): + with pytest.raises(Exception) as excinfo: _gpd_read(str(not_geospatial)) + + assert not isinstance(excinfo.value, UnboundLocalError) diff --git a/uxarray/io/_geopandas.py b/uxarray/io/_geopandas.py index 6fa174f16..8da2ac4f1 100644 --- a/uxarray/io/_geopandas.py +++ b/uxarray/io/_geopandas.py @@ -3,7 +3,6 @@ from uxarray.constants import INT_DTYPE, INT_FILL_VALUE, WGS84_CRS from uxarray.conventions import ugrid -from uxarray.errors import GridInvalidError def _read_geodataframe(filepath, driver=None, **kwargs): @@ -67,13 +66,7 @@ def _gpd_read(filepath, driver=None, **kwargs): import geopandas as gpd - try: - gdf = gpd.read_file(filepath, driver=driver, **kwargs) - except Exception as e: - raise GridInvalidError( - f"Could not read geospatial data from {filepath!r}: {e}" - ) from e - + gdf = gpd.read_file(filepath, driver=driver, **kwargs) gdf = _set_crs(gdf) max_polygon_nodes = gdf["geometry"].apply(_get_num_nodes).max() From c9a85a6b7c31f106a6fe1546dd4e5c3f46bd5de0 Mon Sep 17 00:00:00 2001 From: Rajeev Jain Date: Thu, 20 Aug 2026 13:00:39 -0500 Subject: [PATCH 3/6] Surface IO failures instead of printing or discarding them The geopandas reader and the netCDF fallback shared the same pattern as issue #1693: a failure was caught and reported to stdout, or replaced by a later one, so the actual cause never reached the caller. _open_dataset_with_fallback now chains the fallback engine's error onto the default engine's, so a file that neither engine can open reports both reasons rather than only the second. The two FESOM2 ASCII parsers raised FileNotFoundError("TODO: "), which named neither the missing file nor the directory searched. Assuming WGS84 for CRS-less geospatial data and skipping a geometry type the reader does not support are both warnings now. The latter silently produced a grid with a missing face, which is a wrong result rather than a diagnostic. _is_structured is called speculatively for every dataset before any other format check, so its stdout diagnostics fired for perfectly valid MPAS, Exodus and SCRIP files. They are removed; a negative result is the normal case and _parse_grid_type already raises an actionable error. --- test/core/test_api.py | 15 ++++++++++++++ test/io/test_fesom.py | 16 +++++++++++++++ test/io/test_geopandas.py | 39 +++++++++++++++++++++++++++++++++++++ test/io/test_utils.py | 41 ++++++++++++++++++++++++++++++++++++++- uxarray/core/utils.py | 11 +++++++++-- uxarray/io/_fesom2.py | 10 ++++++++-- uxarray/io/_geopandas.py | 18 ++++++++++++++--- uxarray/io/utils.py | 12 ++++++------ 8 files changed, 148 insertions(+), 14 deletions(-) diff --git a/test/core/test_api.py b/test/core/test_api.py index e9de19936..96421e3b2 100644 --- a/test/core/test_api.py +++ b/test/core/test_api.py @@ -218,6 +218,21 @@ def mock_open_dataset(*args, **kwargs): os.unlink(tmp_path) +def test_open_dataset_with_fallback_chains_both_engine_errors(tmp_path): + """When both engines fail, the fallback error must be chained onto the + default engine's error rather than replacing it.""" + + not_netcdf = tmp_path / "not_netcdf.nc" + not_netcdf.write_text("this is not a netcdf file") + + with pytest.raises(Exception) as excinfo: + _open_dataset_with_fallback(str(not_netcdf)) + + assert excinfo.value.__cause__ is not None, ( + "the default engine's error was discarded" + ) + + def test_list_grid_names_multigrid(gridpath): """List grids from an OASIS-style multi-grid file.""" grid_file = gridpath("scrip", "oasis", "grids.nc") diff --git a/test/io/test_fesom.py b/test/io/test_fesom.py index b58109b26..6c33f69e3 100644 --- a/test/io/test_fesom.py +++ b/test/io/test_fesom.py @@ -62,3 +62,19 @@ def test_open_mfdataset_pi_path(test_data_dir): assert "n_node" in uxds.dims assert "n_face" in uxds.dims assert len(uxds) == 3 + + +def test_parse_nod2d_missing_file_names_the_file(tmp_path): + """A missing 'nod2d.out' must say which file is missing and where.""" + from uxarray.io._fesom2 import _parse_nod2d + + with pytest.raises(FileNotFoundError, match="nod2d.out"): + _parse_nod2d(str(tmp_path)) + + +def test_parse_elem2d_missing_file_names_the_file(tmp_path): + """A missing 'elem2d.out' must say which file is missing and where.""" + from uxarray.io._fesom2 import _parse_elem2d + + with pytest.raises(FileNotFoundError, match="elem2d.out"): + _parse_elem2d(str(tmp_path)) diff --git a/test/io/test_geopandas.py b/test/io/test_geopandas.py index fd98009db..ac9c98db5 100644 --- a/test/io/test_geopandas.py +++ b/test/io/test_geopandas.py @@ -63,3 +63,42 @@ def test_read_failure_raises(tmp_path): _gpd_read(str(not_geospatial)) assert not isinstance(excinfo.value, UnboundLocalError) + + +def test_set_crs_warns_when_crs_is_missing(): + """Assuming WGS84 for CRS-less data is a guess and must be announced.""" + import pytest + gpd = pytest.importorskip("geopandas") + from shapely.geometry import Polygon + + from uxarray.io._geopandas import _set_crs + + gdf = gpd.GeoDataFrame( + geometry=[Polygon([(0, 0), (1, 0), (1, 1)])], crs=None + ) + + with pytest.warns(UserWarning, match="no CRS"): + out = _set_crs(gdf) + + assert out.crs is not None + + +def test_unsupported_geometry_is_reported(): + """Dropping a geometry silently would yield a grid missing a face with no + indication that anything was skipped.""" + import pytest + gpd = pytest.importorskip("geopandas") + from shapely.geometry import Point, Polygon + + from uxarray.io._geopandas import _extract_geometry_info + + gdf = gpd.GeoDataFrame( + geometry=[Polygon([(0, 0), (1, 0), (1, 1), (0, 0)]), Point(5, 5)], + crs="EPSG:4326", + ) + + with pytest.warns(UserWarning, match="unsupported geometry type"): + node_lon, node_lat, connectivity = _extract_geometry_info(gdf, 4) + + # Only the polygon contributes a face; the point is skipped. + assert connectivity.shape[0] == 1 diff --git a/test/io/test_utils.py b/test/io/test_utils.py index 5ed560cc6..324ef719b 100644 --- a/test/io/test_utils.py +++ b/test/io/test_utils.py @@ -3,7 +3,7 @@ import xarray as xr from uxarray.errors import GridInvalidError -from uxarray.io.utils import _parse_grid_type +from uxarray.io.utils import _is_structured, _parse_grid_type @pytest.mark.parametrize( @@ -64,3 +64,42 @@ def test_parse_grid_type_detects_structured_grid(): def test_parse_grid_type_rejects_incomplete_format_signals(dataset): with pytest.raises(GridInvalidError, match="Failed to parse uxgrid information from xarray.Dataset."): _parse_grid_type(dataset) + + +def test_parse_grid_type_is_quiet_for_non_structured_grids(capsys): + """`_is_structured` runs before every other format check, so an unstructured + grid carrying lat/lon coordinates must not produce spurious output.""" + lat = xr.DataArray( + np.array([[0.0, 1.0], [2.0, 3.0]]), + dims=["y", "x"], + attrs={"standard_name": "latitude"}, + ) + lon = xr.DataArray( + np.array([[0.0, 1.0], [2.0, 3.0]]), + dims=["y", "x"], + attrs={"standard_name": "longitude"}, + ) + ds = xr.Dataset(coords={"lat": lat, "lon": lon}) + + with pytest.raises(GridInvalidError): + _parse_grid_type(ds) + + assert capsys.readouterr().out == "" + + +def test_parse_grid_type_is_quiet_for_irregular_spacing(): + """Irregularly spaced coordinates are simply 'not structured', not an event + worth reporting to stdout.""" + lon = xr.DataArray( + np.array([0.0, 1.0, 4.0]), + dims=["lon"], + attrs={"standard_name": "longitude"}, + ) + lat = xr.DataArray( + np.array([-1.0, 0.0, 1.0]), + dims=["lat"], + attrs={"standard_name": "latitude"}, + ) + structured, _, _ = _is_structured(xr.Dataset(coords={"lon": lon, "lat": lat})) + + assert not structured diff --git a/uxarray/core/utils.py b/uxarray/core/utils.py index bfa6c509b..1d875e380 100644 --- a/uxarray/core/utils.py +++ b/uxarray/core/utils.py @@ -32,11 +32,18 @@ def _open_dataset_with_fallback(filename_or_obj, chunks=None, **kwargs): try: # Try opening with xarray's default read engine return xr.open_dataset(filename_or_obj, chunks=chunks, **kwargs) - except Exception: + except Exception as default_engine_error: # If it fails, use the "netcdf4" engine as backup # Extract engine from kwargs to prevent duplicate parameter error engine = kwargs.pop("engine", "netcdf4") - return xr.open_dataset(filename_or_obj, engine=engine, chunks=chunks, **kwargs) + try: + return xr.open_dataset( + filename_or_obj, engine=engine, chunks=chunks, **kwargs + ) + except Exception as fallback_error: + # Chain the fallback onto the original so both engines' reasons are + # visible; otherwise the default engine's error is lost entirely. + raise fallback_error from default_engine_error def _map_dims_to_ugrid( diff --git a/uxarray/io/_fesom2.py b/uxarray/io/_fesom2.py index 3d04806a7..a7f540ef2 100644 --- a/uxarray/io/_fesom2.py +++ b/uxarray/io/_fesom2.py @@ -89,7 +89,10 @@ def _parse_nod2d(grid_path): file_path = os.path.join(grid_path, "nod2d.out") if not os.path.isfile(file_path): - raise FileNotFoundError("TODO: ") + raise FileNotFoundError( + f"Expected a FESOM2 ASCII grid directory containing 'nod2d.out', " + f"but no such file exists under {grid_path!r}." + ) nodes = pd.read_csv( file_path, @@ -120,7 +123,10 @@ def _parse_elem2d(grid_path): """ file_path = os.path.join(grid_path, "elem2d.out") if not os.path.isfile(file_path): - raise FileNotFoundError("TODO: ") + raise FileNotFoundError( + f"Expected a FESOM2 ASCII grid directory containing 'elem2d.out', " + f"but no such file exists under {grid_path!r}." + ) file_content = pd.read_csv( file_path, diff --git a/uxarray/io/_geopandas.py b/uxarray/io/_geopandas.py index 8da2ac4f1..e2be8b65f 100644 --- a/uxarray/io/_geopandas.py +++ b/uxarray/io/_geopandas.py @@ -1,3 +1,5 @@ +import warnings + import numpy as np import xarray as xr @@ -89,11 +91,14 @@ def _set_crs(gdf): """ if gdf.crs is None: gdf = gdf.set_crs(WGS84_CRS) - print("Original CRS: None\nAssigned CRS:", gdf.crs) + warnings.warn( + f"The geospatial data declares no CRS; assuming {WGS84_CRS}. " + f"Coordinates will be wrong if the source uses a different CRS.", + stacklevel=2, + ) if gdf.crs != WGS84_CRS: gdf = gdf.to_crs(WGS84_CRS) - print("Transformed CRS:", gdf.crs) return gdf @@ -134,7 +139,14 @@ def _extract_geometry_info(gdf, max_coord_size): geometry, node_lat_list, node_lon_list, connectivity, node_index ) else: - print(f"Unsupported geometry type: {geometry.geom_type}") + # Skipping a geometry silently would yield a grid that is missing + # faces without any indication that data was dropped. + warnings.warn( + f"Skipping unsupported geometry type {geometry.geom_type!r}; " + f"only Polygon and MultiPolygon are read. The resulting grid " + f"will not contain a face for this geometry.", + stacklevel=2, + ) # Convert lists to numpy arrays at the end node_lon = np.array(node_lon_list) diff --git a/uxarray/io/utils.py b/uxarray/io/utils.py index bf1e1ed77..c95e01dcf 100644 --- a/uxarray/io/utils.py +++ b/uxarray/io/utils.py @@ -146,6 +146,12 @@ def _is_structured(dataset: xr.Dataset, tol: float = 1e-5) -> bool: bool True if the dataset is structured with regularly spaced latitude and longitude, False otherwise. + + Note + ---- + ``_parse_grid_type`` calls this speculatively for every dataset, so a + negative result is the normal case for all other grid formats. It must stay + quiet rather than reporting why the dataset is not structured. """ # Extract all 'standard_name' attributes in lower case standard_names = [ @@ -176,7 +182,6 @@ def _is_structured(dataset: xr.Dataset, tol: float = 1e-5) -> bool: # Ensure that latitude and longitude are one-dimensional if lat.ndim != 1 or lon.ndim != 1: - print("Latitude and/or longitude coordinates are not one-dimensional.") return False, None, None # Calculate the differences between consecutive latitude and longitude values @@ -187,11 +192,6 @@ def _is_structured(dataset: xr.Dataset, tol: float = 1e-5) -> bool: lat_regular = np.all(np.abs(lat_diffs - lat_diffs[0]) <= tol) lon_regular = np.all(np.abs(lon_diffs - lon_diffs[0]) <= tol) - if not lat_regular: - print("Latitude coordinates are not regularly spaced.") - if not lon_regular: - print("Longitude coordinates are not regularly spaced.") - return lat_regular and lon_regular, lon_name, lat_name From 5dbfaae0924522dc7b079bc71fc3e70b66950738 Mon Sep 17 00:00:00 2001 From: Rajeev Jain Date: Fri, 21 Aug 2026 15:09:56 -0500 Subject: [PATCH 4/6] Address review nits: import placement, drop importorskip, absolute paths Move test-local imports to module scope, drop the importorskip calls now that the full test suite always runs with all dependencies installed, and report absolute paths in fesom2's missing-file errors so users with same-named subfolders under different run directories aren't confused by identical-looking relative messages. --- test/io/test_fesom.py | 5 +---- test/io/test_geopandas.py | 20 +++++--------------- uxarray/io/_fesom2.py | 4 ++-- 3 files changed, 8 insertions(+), 21 deletions(-) diff --git a/test/io/test_fesom.py b/test/io/test_fesom.py index 6c33f69e3..c563d1238 100644 --- a/test/io/test_fesom.py +++ b/test/io/test_fesom.py @@ -4,6 +4,7 @@ import pytest import uxarray as ux +from uxarray.io._fesom2 import _parse_elem2d, _parse_nod2d @@ -66,15 +67,11 @@ def test_open_mfdataset_pi_path(test_data_dir): def test_parse_nod2d_missing_file_names_the_file(tmp_path): """A missing 'nod2d.out' must say which file is missing and where.""" - from uxarray.io._fesom2 import _parse_nod2d - with pytest.raises(FileNotFoundError, match="nod2d.out"): _parse_nod2d(str(tmp_path)) def test_parse_elem2d_missing_file_names_the_file(tmp_path): """A missing 'elem2d.out' must say which file is missing and where.""" - from uxarray.io._fesom2 import _parse_elem2d - with pytest.raises(FileNotFoundError, match="elem2d.out"): _parse_elem2d(str(tmp_path)) diff --git a/test/io/test_geopandas.py b/test/io/test_geopandas.py index ac9c98db5..b91ba06b7 100644 --- a/test/io/test_geopandas.py +++ b/test/io/test_geopandas.py @@ -1,4 +1,8 @@ +import geopandas as gpd import numpy as np +import pytest +from shapely.geometry import Point, Polygon + import uxarray as ux def test_read_shpfile(test_data_dir): @@ -46,14 +50,8 @@ def test_read_failure_raises(tmp_path): """A read failure must surface the backend's own error rather than being printed and swallowed into an UnboundLocalError. - Regression test for issue #1693. Requires a geopandas file-IO backend, - otherwise read_file raises ImportError and the test would pass for the - wrong reason. + Regression test for issue #1693. """ - import pytest - - pytest.importorskip("pyogrio") - from uxarray.io._geopandas import _gpd_read not_geospatial = tmp_path / "not_geospatial.shp" @@ -67,10 +65,6 @@ def test_read_failure_raises(tmp_path): def test_set_crs_warns_when_crs_is_missing(): """Assuming WGS84 for CRS-less data is a guess and must be announced.""" - import pytest - gpd = pytest.importorskip("geopandas") - from shapely.geometry import Polygon - from uxarray.io._geopandas import _set_crs gdf = gpd.GeoDataFrame( @@ -86,10 +80,6 @@ def test_set_crs_warns_when_crs_is_missing(): def test_unsupported_geometry_is_reported(): """Dropping a geometry silently would yield a grid missing a face with no indication that anything was skipped.""" - import pytest - gpd = pytest.importorskip("geopandas") - from shapely.geometry import Point, Polygon - from uxarray.io._geopandas import _extract_geometry_info gdf = gpd.GeoDataFrame( diff --git a/uxarray/io/_fesom2.py b/uxarray/io/_fesom2.py index a7f540ef2..de5420374 100644 --- a/uxarray/io/_fesom2.py +++ b/uxarray/io/_fesom2.py @@ -91,7 +91,7 @@ def _parse_nod2d(grid_path): if not os.path.isfile(file_path): raise FileNotFoundError( f"Expected a FESOM2 ASCII grid directory containing 'nod2d.out', " - f"but no such file exists under {grid_path!r}." + f"but no such file exists under {os.path.abspath(grid_path)!r}." ) nodes = pd.read_csv( @@ -125,7 +125,7 @@ def _parse_elem2d(grid_path): if not os.path.isfile(file_path): raise FileNotFoundError( f"Expected a FESOM2 ASCII grid directory containing 'elem2d.out', " - f"but no such file exists under {grid_path!r}." + f"but no such file exists under {os.path.abspath(grid_path)!r}." ) file_content = pd.read_csv( From cd51a6236996d3fbbe9a2514be8a64c5e20d94b5 Mon Sep 17 00:00:00 2001 From: Rajeev Jain Date: Mon, 24 Aug 2026 19:05:31 -0500 Subject: [PATCH 5/6] Move geopandas test helper imports to module top --- test/io/test_geopandas.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/test/io/test_geopandas.py b/test/io/test_geopandas.py index b91ba06b7..6c1fe9181 100644 --- a/test/io/test_geopandas.py +++ b/test/io/test_geopandas.py @@ -4,6 +4,7 @@ from shapely.geometry import Point, Polygon import uxarray as ux +from uxarray.io._geopandas import _extract_geometry_info, _gpd_read, _set_crs def test_read_shpfile(test_data_dir): """Read a shapefile.""" @@ -52,8 +53,6 @@ def test_read_failure_raises(tmp_path): Regression test for issue #1693. """ - from uxarray.io._geopandas import _gpd_read - not_geospatial = tmp_path / "not_geospatial.shp" not_geospatial.write_text("this is not a shapefile") @@ -65,8 +64,6 @@ def test_read_failure_raises(tmp_path): def test_set_crs_warns_when_crs_is_missing(): """Assuming WGS84 for CRS-less data is a guess and must be announced.""" - from uxarray.io._geopandas import _set_crs - gdf = gpd.GeoDataFrame( geometry=[Polygon([(0, 0), (1, 0), (1, 1)])], crs=None ) @@ -80,8 +77,6 @@ def test_set_crs_warns_when_crs_is_missing(): def test_unsupported_geometry_is_reported(): """Dropping a geometry silently would yield a grid missing a face with no indication that anything was skipped.""" - from uxarray.io._geopandas import _extract_geometry_info - gdf = gpd.GeoDataFrame( geometry=[Polygon([(0, 0), (1, 0), (1, 1), (0, 0)]), Point(5, 5)], crs="EPSG:4326", From 233b49762cb4d8cc80440add395b2069478f2c4e Mon Sep 17 00:00:00 2001 From: Rajeev Jain Date: Fri, 28 Aug 2026 13:45:49 -0500 Subject: [PATCH 6/6] Attribute geopandas reader warnings to the caller and name the file Both warnings raised while reading geospatial data used stacklevel=2, which from _set_crs lands on _gpd_read and from _extract_geometry_info on _read_geodataframe -- so the traceback stopped inside uxarray/io/_geopandas.py rather than at the line the user wrote. The correct depths are 5 and 4 today, but hardcoding either only holds until a helper is added or removed, so find_stack_level() counts frames until the stack leaves the package instead. Neither warning said which file it was about; both now name the filepath they were read from. --- test/io/test_geopandas.py | 28 +++++++++++++++++++ test/utils/test_warnings.py | 55 +++++++++++++++++++++++++++++++++++++ uxarray/io/_geopandas.py | 31 ++++++++++++++------- uxarray/utils/warnings.py | 41 +++++++++++++++++++++++++++ 4 files changed, 145 insertions(+), 10 deletions(-) create mode 100644 test/utils/test_warnings.py create mode 100644 uxarray/utils/warnings.py diff --git a/test/io/test_geopandas.py b/test/io/test_geopandas.py index 6c1fe9181..c84026100 100644 --- a/test/io/test_geopandas.py +++ b/test/io/test_geopandas.py @@ -74,6 +74,34 @@ def test_set_crs_warns_when_crs_is_missing(): assert out.crs is not None +def test_read_warnings_name_the_file_and_blame_the_caller(tmp_path): + """A warning about a file has to say which file, and point at the line the + user wrote rather than at whichever uxarray helper happens to raise it. + + A hardcoded ``stacklevel`` gets the second half wrong: from ``_set_crs`` it + lands on ``_gpd_read``, so the traceback stops inside ``_geopandas.py``. + """ + no_crs = tmp_path / "no_crs.shp" + gpd.GeoDataFrame( + geometry=[Polygon([(0, 0), (1, 0), (1, 1), (0, 0)])], crs=None + ).to_file(no_crs) + + mixed = tmp_path / "mixed.geojson" + gpd.GeoDataFrame( + geometry=[Polygon([(0, 0), (1, 0), (1, 1), (0, 0)]), Point(5, 5)], + crs="EPSG:4326", + ).to_file(mixed, driver="GeoJSON") + + for path, match in ((no_crs, "no CRS"), (mixed, "unsupported geometry type")): + with pytest.warns(UserWarning, match=match) as record: + ux.Grid.from_file(str(path), backend="geopandas") + + warning = [w for w in record if match in str(w.message)][0] + assert path.name in str(warning.message) + # Attributed to this test, not to uxarray/io/_geopandas.py. + assert warning.filename == __file__ + + def test_unsupported_geometry_is_reported(): """Dropping a geometry silently would yield a grid missing a face with no indication that anything was skipped.""" diff --git a/test/utils/test_warnings.py b/test/utils/test_warnings.py new file mode 100644 index 000000000..7a283990b --- /dev/null +++ b/test/utils/test_warnings.py @@ -0,0 +1,55 @@ +import warnings + +import geopandas as gpd +from shapely.geometry import Polygon + +import uxarray as ux +from uxarray.io._geopandas import _gpd_read, _set_crs +from uxarray.utils.warnings import find_stack_level + + +def test_find_stack_level_is_independent_of_caller_depth(): + """The level must name the caller's frame however many of its own frames + sit between it and the ``warnings.warn`` call.""" + + def one_frame_deep(): + warnings.warn("boom", stacklevel=find_stack_level()) + + def two_frames_deep(): + one_frame_deep() + + for call in (one_frame_deep, two_frames_deep): + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + call() + # Both helpers live outside uxarray, so the warning belongs to whichever + # of them called ``warn`` -- this file either way. + assert caught[0].filename == __file__ + + +def test_find_stack_level_skips_however_many_internal_frames(tmp_path): + """Entering the reader at different depths must not move the blame. + + ``_set_crs`` raises the warning with one uxarray frame on the stack when + called directly, two through ``_gpd_read``, and four through + ``Grid.from_file``. A hardcoded ``stacklevel`` can only be right for one of + the three. + """ + polygon = Polygon([(0, 0), (1, 0), (1, 1), (0, 0)]) + + no_crs = tmp_path / "no_crs.shp" + gpd.GeoDataFrame(geometry=[polygon], crs=None).to_file(no_crs) + + entry_points = ( + lambda: _set_crs(gpd.GeoDataFrame(geometry=[polygon], crs=None)), + lambda: _gpd_read(str(no_crs)), + lambda: ux.Grid.from_file(str(no_crs), backend="geopandas"), + ) + + for call in entry_points: + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + call() + crs_warnings = [w for w in caught if "no CRS" in str(w.message)] + assert len(crs_warnings) == 1 + assert crs_warnings[0].filename == __file__ diff --git a/uxarray/io/_geopandas.py b/uxarray/io/_geopandas.py index e2be8b65f..4bfb7e022 100644 --- a/uxarray/io/_geopandas.py +++ b/uxarray/io/_geopandas.py @@ -5,6 +5,7 @@ from uxarray.constants import INT_DTYPE, INT_FILL_VALUE, WGS84_CRS from uxarray.conventions import ugrid +from uxarray.utils.warnings import find_stack_level def _read_geodataframe(filepath, driver=None, **kwargs): @@ -29,7 +30,9 @@ def _read_geodataframe(filepath, driver=None, **kwargs): gdf, max_coord_size = _gpd_read(filepath, driver=driver, **kwargs) - node_lon, node_lat, connectivity = _extract_geometry_info(gdf, max_coord_size) + node_lon, node_lat, connectivity = _extract_geometry_info( + gdf, max_coord_size, filepath=filepath + ) grid_ds["node_lon"] = xr.DataArray( data=node_lon, dims=ugrid.NODE_DIM, attrs=ugrid.NODE_LON_ATTRS @@ -69,20 +72,23 @@ def _gpd_read(filepath, driver=None, **kwargs): import geopandas as gpd gdf = gpd.read_file(filepath, driver=driver, **kwargs) - gdf = _set_crs(gdf) + gdf = _set_crs(gdf, filepath=filepath) max_polygon_nodes = gdf["geometry"].apply(_get_num_nodes).max() return gdf, max_polygon_nodes -def _set_crs(gdf): +def _set_crs(gdf, filepath=None): """Set CRS for GeoDataFrame if not already set. Parameters ---------- gdf : gpd.GeoDataFrame GeoDataFrame to set CRS for. + filepath : str, optional + Path the GeoDataFrame was read from, named in the warning below so the + message identifies which file is missing a CRS. Returns ------- @@ -91,10 +97,11 @@ def _set_crs(gdf): """ if gdf.crs is None: gdf = gdf.set_crs(WGS84_CRS) + source = "" if filepath is None else f" in {filepath}" warnings.warn( - f"The geospatial data declares no CRS; assuming {WGS84_CRS}. " + f"The geospatial data{source} declares no CRS; assuming {WGS84_CRS}. " f"Coordinates will be wrong if the source uses a different CRS.", - stacklevel=2, + stacklevel=find_stack_level(), ) if gdf.crs != WGS84_CRS: @@ -103,7 +110,7 @@ def _set_crs(gdf): return gdf -def _extract_geometry_info(gdf, max_coord_size): +def _extract_geometry_info(gdf, max_coord_size, filepath=None): """Extract node and connectivity information from GeoDataFrame. Parameters @@ -112,6 +119,9 @@ def _extract_geometry_info(gdf, max_coord_size): GeoDataFrame with geometries. max_coord_size : int Maximum number of nodes in a polygon/multipolygon. + filepath : str, optional + Path the GeoDataFrame was read from, named in the skipped-geometry + warning below so the message identifies which file dropped a face. Returns ------- @@ -141,11 +151,12 @@ def _extract_geometry_info(gdf, max_coord_size): else: # Skipping a geometry silently would yield a grid that is missing # faces without any indication that data was dropped. + source = "" if filepath is None else f" in {filepath}" warnings.warn( - f"Skipping unsupported geometry type {geometry.geom_type!r}; " - f"only Polygon and MultiPolygon are read. The resulting grid " - f"will not contain a face for this geometry.", - stacklevel=2, + f"Skipping unsupported geometry type {geometry.geom_type!r}" + f"{source}; only Polygon and MultiPolygon are read. The " + f"resulting grid will not contain a face for this geometry.", + stacklevel=find_stack_level(), ) # Convert lists to numpy arrays at the end diff --git a/uxarray/utils/warnings.py b/uxarray/utils/warnings.py new file mode 100644 index 000000000..15470547a --- /dev/null +++ b/uxarray/utils/warnings.py @@ -0,0 +1,41 @@ +import inspect +import os +from functools import lru_cache + + +@lru_cache(maxsize=None) +def _package_dir(): + import uxarray + + return os.path.dirname(os.path.abspath(uxarray.__file__)) + + +def find_stack_level(): + """``stacklevel`` that attributes a warning to the first frame outside uxarray. + + A hardcoded ``stacklevel`` names whichever frame happens to sit that many + levels up, so a warning raised deep in a reader either blames another + internal module or silently starts pointing somewhere else the moment a + helper is added or removed. Counting frames until the stack leaves the + package keeps the warning pinned to the caller's own line regardless of how + many internal calls separate them. + """ + package_dir = _package_dir() + + frame = inspect.currentframe() + n = 0 + try: + while frame is not None: + if os.path.abspath(frame.f_code.co_filename).startswith(package_dir): + frame = frame.f_back + n += 1 + else: + break + finally: + # Break the reference cycle a live frame object creates. + del frame + + # ``n`` counts this function's own frame, which is exactly the off-by-one + # between "frames inside the package" and the ``stacklevel`` the caller of + # ``warnings.warn`` needs. + return n