[XARRAY] add env settings to fallback to WGS84 when no CRS is provided #725
Conversation
There was a problem hiding this comment.
The config option makes the behaviour nicely explicit.
I would probably recommend to pretty much all users to turn this WGS84 default option on, since rectilinear lat/lon grids are so very common for Xarray datasets (climate data in NetCDF), so it is a pretty safe guess. One could add some heuristics around it, too, but that is not rio-tiler material.
Had a few questions regarding the implementation, please see below.
| if not self.crs: | ||
| warnings.warn( | ||
| "Dataset doesn't have a valid CRS set. rio-tiler might not work properly", | ||
| MissingCRSWarning, | ||
| ) |
There was a problem hiding this comment.
Why do we not throw an exception in this case?
Using an XarrayTiler for anything meaningful without a (valid) CRS, will inevitably fail.
There was a problem hiding this comment.
We usually don't fail for this because you can still access the dataset 🤷
Let me think about it
| "Dataset doesn't have a valid CRS set. Automatically setting CRS to WGS84", | ||
| OverwritingCRSWarning, | ||
| ) | ||
| self.input.rio.write_crs("epsg:4326", inplace=True) |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
😬 sadly rio-tiler gets the input directly from the user so we can't really avoid editing the origin data array
🤔 Well I usually don't like magic, meaning if would prefer that people know what they are doing instead of trying to patch for every issues. IMO not having CRS set in their dataset is their issue, and shouldn't be rio-tiler one. As think what we do in titiler-xarray is better https://github.com/developmentseed/titiler-xarray/blob/dev/titiler/xarray/reader.py#L170-L172 |
|
in #726 I've proposed another approach which raises errors instead of trying to fix the dataset and then expect the |
closes #717
cc @j08lue