Inconsistent and non-standard CRS handling for geometry arguments in DataCube
stumbled on this while working on #104: https://github.com/Open-EO/openeo-python-client/blob/5a3e6f40cf5945ea885165db9845587435be66f0/openeo/rest/datacube.py#L1074-L1087
e.g. public facing from cube.aggregate_spatial(..., crs=...) and cube.mask_polygon(..., srs=...)
This CRS handling has some problems:
- it's an ad-hoc, non-standard feature not present in official processes
- it's inconsistent: sometimes it will apply (when geometry is client-side GeoJSON), sometimes it won't (e.g. back-end side vector cube)
- it will ignore crs info already present in GeoJSON and will blindly overwrite it
I think we should get rid of this
related discussions and threads:
- https://github.com/Open-EO/openeo-processes/issues/235
- #204
- e344c4c105e8d5b13e16d99bc19baeb92ca79cf0
- https://github.com/Open-EO/openeo-python-client/pull/202
As far as I understand the threads above, the "crs" support mentioned above was originally targeted to the use case where the geometry is provided as shapely object (which has no explicit CRS info), which has to be converted to GeoJSON to be embedded in the process graph. Over time, a lot more geometry value types for geometries were added (e.g UDP parameters, vector cubes, load_url, ...), where it's impossible to attach that CRS to, so we now have a very inconsistent CRS handling situation.
proposed solution:
- stop supporting
crs/srsarguments directly inaggregate_spatial, ... (because it's impossible to do it consistently). Instead throw exception pointing to proper way to do it (e.g. docs). - provide convenience function to support the conversion need from shapely to GeoJSON with custom CRS
So use cases that wanted to use shapely geometries with custom crs the old way:
cube = cube.aggregate_spatial(
geometries=shapely_geometry,
crs="EPSG:123456",
...
)
would have to change to something like
cube = cube.aggregate_spatial(
geometries=shapely_to_geojson(shapely_geometry, crs="EPSG:123456"),
...
)
I would:
- make sure that shapely geometry remains supported
- add shapely geometry support to load_collection as well, the absence there is the most annoying inconsistency for me
- improve documentation of crs keyword to state that it's actually deprecated.