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.

Sign up for GitHub

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

Jump to bottom

apply_polygon (previously known as chunk_polygon) abuses MultiPolygon as collection of polygons in its internal API #288

Open
soxofaan opened this issue Jun 7, 2024 · 1 comment · May be fixed by Open-EO/openeo-geopyspark-driver#801 or #291
Open

apply_polygon (previously known as chunk_polygon) abuses MultiPolygon as collection of polygons in its internal API #288

soxofaan opened this issue Jun 7, 2024 · 1 comment · May be fixed by Open-EO/openeo-geopyspark-driver#801 or #291
Assignees
@JeroenVerstraelen
Labels
technical debt

Comments

@soxofaan
Copy link
Member

soxofaan commented Jun 7, 2024 •
edited
Loading

Stumbled on this while working on #287:

The apply_polygon process implementation (originally added as chunk_polygon in a411109) coerces the provided geometries into a MultiPolygon :

openeo-python-driver/openeo_driver/ProcessGraphDeserializer.py

Lines 859 to 874 in ea078ff

# TODO: this logic (copied from original chunk_polygon implementation) coerces the input polygons
# to a single MultiPolygon of pure (non-multi) polygons, which is conceptually wrong.
# Instead it should normalize to a feature collection or vector cube.
if isinstance(polygons, DelayedVector):
polygons = list(polygons.geometries)
for p in polygons:
if not isinstance(p, shapely.geometry.Polygon):
reason = "{m!s} is not a polygon.".format(m=p)
raise ProcessParameterInvalidException(parameter="polygons", process="apply_polygon", reason=reason)
polygon = MultiPolygon(polygons)
elif isinstance(polygons, shapely.geometry.base.BaseGeometry):
polygon = MultiPolygon(polygons)
elif isinstance(polygons, dict):
polygon = geojson_to_multipolygon(polygons)
if isinstance(polygon, shapely.geometry.Polygon):
polygon = MultiPolygon([polygon])

and pushes this API through into geopyspark driver, e.g.
https://github.com/Open-EO/openeo-geopyspark-driver/blob/1f22c0da34535e986908480673ad341c9f89324d/openeogeotrellis/geopysparkdatacube.py#L572-L575

MultiPolygon should not be abused as a collection of polygons. Instead the API should use something like a FeatureCollection or vector cube abstraction
@soxofaan
Copy link
Member Author

soxofaan commented Jun 7, 2024

related to (or causing) this confusion: the apply_polygon process should use argument geometries instead of polygons -> Open-EO/openeo-processes#511

This was referenced Jun 7, 2024
Open
Open
soxofaan added a commit that referenced this issue Jun 7, 2024
@soxofaan
Issue #288/#229 add TODO notes about wrong MultiPolygon usage
605f153
soxofaan added a commit that referenced this issue Jun 7, 2024
@soxofaan
Issue #287 Support DriverVectorCube in apply_polygon
c95d1f1 
other refs: #288, d180c24
@soxofaan soxofaan changed the title apply_polygon (previously known as chunk_polygon) abuses MultiPolygon as collection of polygons in it's internal API apply_polygon (previously known as chunk_polygon) abuses MultiPolygon as collection of polygons in its internal API Jun 7, 2024
soxofaan added a commit that referenced this issue Jun 8, 2024
@soxofaan
Issue #287 More support for DriverVectorCube in apply_polygon
6f6fa8b 
other refs: #288, d180c24
JeroenVerstraelen added a commit that referenced this issue Jun 9, 2024
@JeroenVerstraelen
Issue #288 use DriverVectorCube in apply_polygon
3445dec
JeroenVerstraelen added a commit to Open-EO/openeo-geopyspark-driver that referenced this issue Jun 9, 2024
@JeroenVerstraelen
use DriverVectorCube in chunk_polygon Open-EO/openeo-python-driver#288
eed1489
This was linked to pull requests Jun 10, 2024
use DriverVectorCube in chunk_polygon Open-EO/openeo-python-driver#288 Open-EO/openeo-geopyspark-driver#801
Open
Issue #288 use DriverVectorCube in apply_polygon #291
Open
soxofaan added a commit to Open-EO/openeo-geopyspark-driver that referenced this issue Jun 11, 2024
@soxofaan
quickfix for DriverVectorCube support in chunk_polygon
0492296 
refs: Open-EO/openeo-python-driver#287, Open-EO/openeo-python-driver#288, Open-EO/openeo-python-driver#291 #801
@JeroenVerstraelen JeroenVerstraelen self-assigned this Jun 17, 2024
soxofaan added a commit that referenced this issue Jul 17, 2024
@soxofaan
apply_polygon: rename polygons parameter to geometries
15b72a7 
But still support legacy `polygons` for now

refs: Open-EO/openeo-processes#511, #288
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@JeroenVerstraelen JeroenVerstraelen

Labels
technical debt
Projects
None yet
Milestone
No milestone
2 participants
@soxofaan @JeroenVerstraelen