From e2760400c22b241e93c56e57563826943e9d718d Mon Sep 17 00:00:00 2001 From: vincentsarago Date: Mon, 2 Sep 2024 22:08:51 +0200 Subject: [PATCH 1/6] raise error when xarray dataset have wrong geo information --- CHANGES.md | 1 + docs/src/readers.md | 2 +- rio_tiler/errors.py | 8 +++++++ rio_tiler/io/xarray.py | 19 ++++++++++++++- tests/test_io_xarray.py | 51 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 79 insertions(+), 2 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 868db7cd..9c76940b 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,6 @@ # unreleased +* raise `MissingCRS` or `InvalidGeographicBounds` errors when Xarray datasets have wrong geographic metadata * better error message for `TileOutsideBounds` errors (author @abarciauskas-bgse, https://github.com/cogeotiff/rio-tiler/pull/712) * handle of inverted latitude in `reader.point` (author @georgespill, https://github.com/cogeotiff/rio-tiler/pull/716) diff --git a/docs/src/readers.md b/docs/src/readers.md index 4d484056..db04e5be 100644 --- a/docs/src/readers.md +++ b/docs/src/readers.md @@ -1179,4 +1179,4 @@ print(info.json(exclude_none=True)) !!! Important Not Implemented -``` + diff --git a/rio_tiler/errors.py b/rio_tiler/errors.py index 1fec6a8a..44373533 100644 --- a/rio_tiler/errors.py +++ b/rio_tiler/errors.py @@ -83,3 +83,11 @@ class AssetAsBandError(RioTilerError): class InvalidPointDataError(RioTilerError): """Invalid PointData.""" + + +class MissingCRS(RioTilerError): + """Dataset doesn't have CRS information.""" + + +class InvalidGeographicBounds(RioTilerError): + """Invalid Geographic bounds.""" diff --git a/rio_tiler/io/xarray.py b/rio_tiler/io/xarray.py index c4bb323b..30a404da 100644 --- a/rio_tiler/io/xarray.py +++ b/rio_tiler/io/xarray.py @@ -15,7 +15,12 @@ from rasterio.warp import transform as transform_coords from rio_tiler.constants import WEB_MERCATOR_TMS, WGS84_CRS -from rio_tiler.errors import PointOutsideBounds, TileOutsideBounds +from rio_tiler.errors import ( + InvalidGeographicBounds, + MissingCRS, + PointOutsideBounds, + TileOutsideBounds, +) from rio_tiler.io.base import BaseReader from rio_tiler.models import BandStatistics, ImageData, Info, PointData from rio_tiler.types import BBox, NoData, WarpResampling @@ -72,6 +77,18 @@ def __attrs_post_init__(self): self.bounds = tuple(self.input.rio.bounds()) self.crs = self.input.rio.crs + if not self.crs: + raise MissingCRS( + "Dataset doesn't have CRS information, please add it before using rio-tiler (e.g. `ds.rio.write_crs('epsg:4326', inplace=True)`)" + ) + + if self.crs == WGS84_CRS and ( + self.bounds[0] < -180 + or self.bounds[1] < -90 + or self.bounds[2] > 180 + or self.bounds[3] > 90 + ): + raise InvalidGeographicBounds(f"Invalid geographic bounds: {self.bounds}") self._dims = [ d diff --git a/tests/test_io_xarray.py b/tests/test_io_xarray.py index 1d128f93..a75c098a 100644 --- a/tests/test_io_xarray.py +++ b/tests/test_io_xarray.py @@ -8,6 +8,7 @@ import rioxarray import xarray +from rio_tiler.errors import InvalidGeographicBounds, MissingCRS from rio_tiler.io import XarrayReader @@ -321,3 +322,53 @@ def test_xarray_reader_resampling(): with pytest.warns(DeprecationWarning): _ = dst.feature(feat, resampling_method="nearest") + + +def test_xarray_reader_no_crs(): + """Should raise MissingCRS.""" + arr = numpy.arange(0.0, 33 * 35).reshape(1, 33, 35) + data = xarray.DataArray( + arr, + dims=("time", "y", "x"), + coords={ + "x": list(range(-170, 180, 10)), + "y": list(range(-80, 85, 5)), + "time": [datetime(2022, 1, 1)], + }, + ) + data.attrs.update({"valid_min": arr.min(), "valid_max": arr.max()}) + with pytest.raises(MissingCRS): + with XarrayReader(data): + pass + + +def test_xarray_reader_invalid_bounds_crs(): + """Should raise InvalidGeographicBounds.""" + arr = numpy.arange(0.0, 33 * 35).reshape(1, 33, 35) + data = xarray.DataArray( + arr, + dims=("time", "y", "x"), + coords={ + "x": list(range(10, 360, 10)), + "y": list(range(-80, 85, 5)), + "time": [datetime(2022, 1, 1)], + }, + ) + data.rio.write_crs("epsg:4326", inplace=True) + with pytest.raises(InvalidGeographicBounds): + with XarrayReader(data): + pass + + data = xarray.DataArray( + arr, + dims=("time", "y", "x"), + coords={ + "x": list(range(-170, 180, 10)), + "y": list(range(15, 180, 5)), + "time": [datetime(2022, 1, 1)], + }, + ) + data.rio.write_crs("epsg:4326", inplace=True) + with pytest.raises(InvalidGeographicBounds): + with XarrayReader(data): + pass From 71c5ba859e326f49ba8e89fbe4cd980e31d7a7db Mon Sep 17 00:00:00 2001 From: Vincent Sarago Date: Tue, 3 Sep 2024 11:30:31 +0200 Subject: [PATCH 2/6] Update tests/test_io_xarray.py Co-authored-by: Jonas --- tests/test_io_xarray.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_io_xarray.py b/tests/test_io_xarray.py index a75c098a..cdefbd08 100644 --- a/tests/test_io_xarray.py +++ b/tests/test_io_xarray.py @@ -331,8 +331,8 @@ def test_xarray_reader_no_crs(): arr, dims=("time", "y", "x"), coords={ - "x": list(range(-170, 180, 10)), - "y": list(range(-80, 85, 5)), + "x": numpy.arange(-170, 180, 10), + "y": numpy.arange(-80, 85, 5), "time": [datetime(2022, 1, 1)], }, ) From 490d46193f5a61a9f379a77ada871b20922ef8db Mon Sep 17 00:00:00 2001 From: Vincent Sarago Date: Tue, 3 Sep 2024 11:30:57 +0200 Subject: [PATCH 3/6] Update rio_tiler/io/xarray.py Co-authored-by: Jonas --- rio_tiler/io/xarray.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rio_tiler/io/xarray.py b/rio_tiler/io/xarray.py index 30a404da..98023e77 100644 --- a/rio_tiler/io/xarray.py +++ b/rio_tiler/io/xarray.py @@ -84,9 +84,9 @@ def __attrs_post_init__(self): if self.crs == WGS84_CRS and ( self.bounds[0] < -180 - or self.bounds[1] < -90 + or min(self.bounds[1], self.bounds[3]) < -90 or self.bounds[2] > 180 - or self.bounds[3] > 90 + or max(self.bounds[1], self.bounds[3]) > 90 ): raise InvalidGeographicBounds(f"Invalid geographic bounds: {self.bounds}") From 2560f820232f4a7c06cf04b4c2f24addf670370c Mon Sep 17 00:00:00 2001 From: vincentsarago Date: Tue, 3 Sep 2024 12:16:16 +0200 Subject: [PATCH 4/6] test ordered bounds --- rio_tiler/io/xarray.py | 5 +++-- tests/test_io_xarray.py | 44 +++++++++++++++++++++++++++++++++-------- 2 files changed, 39 insertions(+), 10 deletions(-) diff --git a/rio_tiler/io/xarray.py b/rio_tiler/io/xarray.py index 98023e77..24c9292c 100644 --- a/rio_tiler/io/xarray.py +++ b/rio_tiler/io/xarray.py @@ -75,6 +75,7 @@ def __attrs_post_init__(self): assert xarray is not None, "xarray must be installed to use XarrayReader" assert rioxarray is not None, "rioxarray must be installed to use XarrayReader" + # NOTE: rioxarray returns **ordered** bounds in form of (minx, miny, maxx, maxx) self.bounds = tuple(self.input.rio.bounds()) self.crs = self.input.rio.crs if not self.crs: @@ -84,9 +85,9 @@ def __attrs_post_init__(self): if self.crs == WGS84_CRS and ( self.bounds[0] < -180 - or min(self.bounds[1], self.bounds[3]) < -90 + or self.bounds[1] < -90 or self.bounds[2] > 180 - or max(self.bounds[1], self.bounds[3]) > 90 + or self.bounds[3] > 90 ): raise InvalidGeographicBounds(f"Invalid geographic bounds: {self.bounds}") diff --git a/tests/test_io_xarray.py b/tests/test_io_xarray.py index cdefbd08..57515a1f 100644 --- a/tests/test_io_xarray.py +++ b/tests/test_io_xarray.py @@ -19,8 +19,8 @@ def test_xarray_reader(): arr, dims=("time", "y", "x"), coords={ - "x": list(range(-170, 180, 10)), - "y": list(range(-80, 85, 5)), + "x": numpy.arange(-170, 180, 10), + "y": numpy.arange(-80, 85, 5), "time": [datetime(2022, 1, 1)], }, ) @@ -269,8 +269,8 @@ def test_xarray_reader_resampling(): arr, dims=("time", "y", "x"), coords={ - "x": list(range(-170, 180, 10)), - "y": list(range(-80, 85, 5)), + "x": numpy.arange(-170, 180, 10), + "y": numpy.arange(-80, 85, 5), "time": [datetime(2022, 1, 1)], }, ) @@ -349,8 +349,22 @@ def test_xarray_reader_invalid_bounds_crs(): arr, dims=("time", "y", "x"), coords={ - "x": list(range(10, 360, 10)), - "y": list(range(-80, 85, 5)), + "x": numpy.arange(10, 360, 10), + "y": numpy.arange(-80, 85, 5), + "time": [datetime(2022, 1, 1)], + }, + ) + data.rio.write_crs("epsg:4326", inplace=True) + with pytest.raises(InvalidGeographicBounds): + with XarrayReader(data): + pass + + data = xarray.DataArray( + arr, + dims=("time", "y", "x"), + coords={ + "x": numpy.arange(-170, 180, 10), + "y": numpy.arange(15, 180, 5), "time": [datetime(2022, 1, 1)], }, ) @@ -363,8 +377,8 @@ def test_xarray_reader_invalid_bounds_crs(): arr, dims=("time", "y", "x"), coords={ - "x": list(range(-170, 180, 10)), - "y": list(range(15, 180, 5)), + "x": numpy.arange(-170, 180, 10), + "y": numpy.arange(15, 180, 5), "time": [datetime(2022, 1, 1)], }, ) @@ -372,3 +386,17 @@ def test_xarray_reader_invalid_bounds_crs(): with pytest.raises(InvalidGeographicBounds): with XarrayReader(data): pass + + # Inverted bounds are still ok because rioxarray reorder the bounds + data = xarray.DataArray( + arr, + dims=("time", "y", "x"), + coords={ + "x": numpy.flip(numpy.arange(-170, 180, 10)), + "y": numpy.flip(numpy.arange(-80, 85, 5)), + "time": [datetime(2022, 1, 1)], + }, + ) + data.rio.write_crs("epsg:4326", inplace=True) + with XarrayReader(data): + pass From 97364225e9f0daba2ca5151fadbc993e9ec54ea4 Mon Sep 17 00:00:00 2001 From: Vincent Sarago Date: Wed, 4 Sep 2024 17:54:24 +0200 Subject: [PATCH 5/6] Update rio_tiler/io/xarray.py Co-authored-by: Jonas --- rio_tiler/io/xarray.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rio_tiler/io/xarray.py b/rio_tiler/io/xarray.py index 24c9292c..32446add 100644 --- a/rio_tiler/io/xarray.py +++ b/rio_tiler/io/xarray.py @@ -89,7 +89,7 @@ def __attrs_post_init__(self): or self.bounds[2] > 180 or self.bounds[3] > 90 ): - raise InvalidGeographicBounds(f"Invalid geographic bounds: {self.bounds}") + raise InvalidGeographicBounds(f"Invalid geographic bounds: {self.bounds}. Must be within (-180, -90, 180, 90).") self._dims = [ d From 375dc1e18a748f2108014071ecba5a9bb9251253 Mon Sep 17 00:00:00 2001 From: vincentsarago Date: Thu, 5 Sep 2024 00:33:23 +0200 Subject: [PATCH 6/6] fix lint --- rio_tiler/io/xarray.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/rio_tiler/io/xarray.py b/rio_tiler/io/xarray.py index 32446add..c0f88585 100644 --- a/rio_tiler/io/xarray.py +++ b/rio_tiler/io/xarray.py @@ -89,7 +89,9 @@ def __attrs_post_init__(self): or self.bounds[2] > 180 or self.bounds[3] > 90 ): - raise InvalidGeographicBounds(f"Invalid geographic bounds: {self.bounds}. Must be within (-180, -90, 180, 90).") + raise InvalidGeographicBounds( + f"Invalid geographic bounds: {self.bounds}. Must be within (-180, -90, 180, 90)." + ) self._dims = [ d