cogeotiff / rio-tiler

User friendly Rasterio plugin to read raster datasets.
https://cogeotiff.github.io/rio-tiler/
BSD 3-Clause "New" or "Revised" License
511 stars 106 forks source link

Error with expression with unicode dtype #695

Closed emmanuelmathot closed 6 months ago

emmanuelmathot commented 6 months ago

This PR fixes #694. It checks that the statistics provided in the STAC item for an asset are valid. If not, a warning is output. @vincentsarago If you consider stats must be valid without warning, drop this PR.

emmanuelmathot commented 6 months ago

error found. Statistics in the stac asset are strings. It is not the first time we encounter this issue. @vincentsarago Should we make a check of the statistics somewhere in STACReader and output a warning in case of non number values?

vincentsarago commented 6 months ago

@emmanuelmathot 🤯 thanks for looking at this 🙏

is it in the specification that the stats could be string? If yes we can add a fix in https://github.com/cogeotiff/rio-tiler/blob/89fac81e011218f64071857a70244976809a48c7/rio_tiler/io/stac.py#L303-L308

emmanuelmathot commented 6 months ago

no it must be a double, this is an issue on our side but it is a common mistake and any warning clue on a error like this is welcome

vincentsarago commented 6 months ago

@emmanuelmathot Sadly rio-tiler cannot account for all error that could happen in STAC 😬

If we start adding patch for some things I'm afraid that we would receive PR for others 🤷

emmanuelmathot commented 6 months ago

I totally understand. My idea was just to check the data in update_statistics method and make a warning if the stac data are not valid but if you think it will create overheads in the code, let's drop it.