Skip to content
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

[XARRAY] add env settings to fallback to WGS84 when no CRS is provided #725

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# unreleased

* add `RIO_TILER_XARRAY_DEFAULT_WGS84=TRUE/FALSE` environment variable to enable setting WGS84 CRS by default when no CRS is provided in Xarray dataset
* 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)

Expand Down
6 changes: 5 additions & 1 deletion docs/src/readers.md
Original file line number Diff line number Diff line change
Expand Up @@ -1179,4 +1179,8 @@ print(info.json(exclude_none=True))
!!! Important

Not Implemented
```


#### Settings

`RIO_TILER_XARRAY_DEFAULT_WGS84=TRUE/FALSE` environment variable can be used to enable setting `WGS84` CRS by default when the CRS information is not defined in the Xarray dataset. Defaults to `False`.
8 changes: 8 additions & 0 deletions rio_tiler/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,3 +83,11 @@ class AssetAsBandError(RioTilerError):

class InvalidPointDataError(RioTilerError):
"""Invalid PointData."""


class MissingCRSWarning(UserWarning):
"""Missing CRS information."""


class OverwritingCRSWarning(UserWarning):
"""Overwriting CRS information."""
26 changes: 25 additions & 1 deletion rio_tiler/io/xarray.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from __future__ import annotations

import os
import warnings
from typing import Any, Dict, List, Optional

Expand All @@ -15,7 +16,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 (
MissingCRSWarning,
OverwritingCRSWarning,
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
Expand All @@ -32,6 +38,11 @@
rioxarray = None # type: ignore


default_to_wgs84_crs = os.environ.get(
"RIO_TILER_XARRAY_DEFAULT_WGS84", "FALSE"
).upper() in ["TRUE", "YES"]


@attr.s
class XarrayReader(BaseReader):
"""Xarray Reader.
Expand Down Expand Up @@ -71,7 +82,20 @@ def __attrs_post_init__(self):
assert rioxarray is not None, "rioxarray must be installed to use XarrayReader"

self.bounds = tuple(self.input.rio.bounds())

if not self.input.rio.crs and default_to_wgs84_crs:
warnings.warn(
"Dataset doesn't have a valid CRS set. Automatically setting CRS to WGS84",
OverwritingCRSWarning,
)
self.input.rio.write_crs("epsg:4326", inplace=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

This mutates the self.input Xarray object (DataArray).

If we do not copy what the user passed in earlier, this behaviour could be unexpected for a user.

If there is a chance that the XarrayReader mutates the input, should we perhaps always have it create a copy up-front?

Copy link
Member Author

Choose a reason for hiding this comment

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

🤔 good point

Copy link
Member Author

Choose a reason for hiding this comment

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

😬 sadly rio-tiler gets the input directly from the user so we can't really avoid editing the origin data array


self.crs = self.input.rio.crs
if not self.crs:
warnings.warn(
"Dataset doesn't have a valid CRS set. rio-tiler might not work properly",
MissingCRSWarning,
)
Comment on lines +94 to +98
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we not throw an exception in this case?

Using an XarrayTiler for anything meaningful without a (valid) CRS, will inevitably fail.

Copy link
Member Author

Choose a reason for hiding this comment

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

We usually don't fail for this because you can still access the dataset 🤷

Let me think about it


self._dims = [
d
Expand Down
50 changes: 50 additions & 0 deletions tests/test_io_xarray.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
"""tests rio_tiler.io.xarray.XarrayReader"""

from datetime import datetime
from unittest.mock import patch

import morecantile
import numpy
import pytest
import rioxarray
import xarray

from rio_tiler.errors import MissingCRSWarning, OverwritingCRSWarning
from rio_tiler.io import XarrayReader


Expand Down Expand Up @@ -321,3 +323,51 @@ def test_xarray_reader_resampling():

with pytest.warns(DeprecationWarning):
_ = dst.feature(feat, resampling_method="nearest")


@patch("rio_tiler.io.xarray.default_to_wgs84_crs", new=True)
def test_xarray_reader_default_crs():
"""Should raise OverwritingCRSWarning warning."""
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.warns(OverwritingCRSWarning):
with XarrayReader(data) as dst:
info = dst.info()
assert info.minzoom == 0
assert info.maxzoom == 0
assert info.band_metadata == [("b1", {})]
assert info.band_descriptions == [("b1", "2022-01-01T00:00:00.000000000")]
assert info.height == 33
assert info.width == 35
assert info.count == 1
assert info.attrs


@patch("rio_tiler.io.xarray.default_to_wgs84_crs", new=False)
def test_xarray_reader_no_default_crs():
"""Should raise MissingCRSWarning warning."""
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.warns(MissingCRSWarning):
with XarrayReader(data) as dst:
assert dst.info()
Loading