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

ENH: Add polygonhull_simplify exposing PolygonHullSimplifier #2050

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

aloi214
Copy link

@aloi214 aloi214 commented May 13, 2024

xref #1570

GEOS introduces a GEOSPolygonHullSimplify and GEOSPolygonHullSimplifyMode in v3.11. GEOSPolygonHullSimplifyMode adds an extra parameter named parameterMode, when parameterMode equals 1 it works same as GEOSPolygonHullSimplify.

This PR adds a shapely binding of GEOSPolygonHullSimplifyMode.

@aloi214
Copy link
Author

aloi214 commented May 28, 2024

@jorisvandenbossche I hope I’m not interrupting you at this time. It's my first PR and I want to know what do I need to do with this PR. 😄

@@ -926,6 +927,61 @@ def simplify(geometry, tolerance, preserve_topology=True, **kwargs):
return lib.simplify(geometry, tolerance, **kwargs)


@multithreading_enabled
def polygonhull_simplify(
Copy link
Contributor

Choose a reason for hiding this comment

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

I suppose that given the aim to match the PostGIS naming, this should be called simplify_polygon_hull after ST_SimplifyPolygonHull.

Comment on lines 964 to 968
if lib.geos_version < (3, 11, 0):
raise UnsupportedGEOSVersionError(
"Polygonhull Simplifier require GEOS >= 3.11.0, "
f"found {lib.geos_version_string}"
)
Copy link
Contributor

Choose a reason for hiding this comment

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

You shall replace this with the @requires_geos("3.11.0") decorator.

Larger values produce less concave results. A value of 1 produces
the convex hull; a value of 0 produces the original geometry.
parameter_mode : str
vertice - Fraction of input vertices retained, area - Ratio of simplified hull area to input area
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that it should be "vertex", not "vertice".

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for your opinion, I'll adjust the name.

@aloi214 aloi214 requested a review from martinfleis May 31, 2024 06:43
):
"""Computes a boundary-respecting hull of a polygonal geometry, with hull
shape determined by a target parameter specifying the fraction of the input
vertices or the input geometry's area retained retained in the result.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
vertices or the input geometry's area retained retained in the result.
vertices or the input geometry's area retained retained in the result.
The simplified geometry is ensured to entirely cover the input geometry if ``is_outer=True``
or to be entirely within the input geometry if ``is_outer=True``.

This is the main distinction compared to normal simplify, so it is worth being explicit about it.

Comment on lines +942 to +945
parameter : float or array_like
Larger values produce less concave results. A value of 1 produces
the convex hull; a value of 0 produces the original geometry.
parameter_mode : str
Copy link
Contributor

Choose a reason for hiding this comment

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

A question is whether this shall be called tolerance like in simplify but that is more up to a discussion.

Comment on lines +955 to +963
>>> from shapely import Polygon, polygonhull_simplify
>>> polygonhull_simplify(Polygon([(0,0), (0, 4), (1,3), (3,4), (10, 4), (10, 0), (0, 0)]), 0.1, "vertice", False)
<POLYGON ((0 0, 10 0, 3 4, 0 0))>
>>> polygonhull_simplify(Polygon([(0,0), (0, 4), (1,3), (3,4), (10, 4), (10, 0), (0, 0)]), 0.1, "vertice", True)
<POLYGON ((0 0, 0 4, 10 4, 10 0, 0 0))>
>>> polygonhull_simplify(Polygon([(0,0), (0, 4), (1,3), (3,4), (10, 4), (10, 0), (0, 0)]), 0.1, "area", False)
<POLYGON ((0 0, 10 0, 10 4, 3 4, 1 3, 0 0))>
>>> polygonhull_simplify(Polygon([(0,0), (0, 4), (1,3), (3,4), (10, 4), (10, 0), (0, 0)]), 0.1, "area", True)
<POLYGON ((0 0, 0 4, 10 4, 10 0, 0 0))>
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
>>> from shapely import Polygon, polygonhull_simplify
>>> polygonhull_simplify(Polygon([(0,0), (0, 4), (1,3), (3,4), (10, 4), (10, 0), (0, 0)]), 0.1, "vertice", False)
<POLYGON ((0 0, 10 0, 3 4, 0 0))>
>>> polygonhull_simplify(Polygon([(0,0), (0, 4), (1,3), (3,4), (10, 4), (10, 0), (0, 0)]), 0.1, "vertice", True)
<POLYGON ((0 0, 0 4, 10 4, 10 0, 0 0))>
>>> polygonhull_simplify(Polygon([(0,0), (0, 4), (1,3), (3,4), (10, 4), (10, 0), (0, 0)]), 0.1, "area", False)
<POLYGON ((0 0, 10 0, 10 4, 3 4, 1 3, 0 0))>
>>> polygonhull_simplify(Polygon([(0,0), (0, 4), (1,3), (3,4), (10, 4), (10, 0), (0, 0)]), 0.1, "area", True)
<POLYGON ((0 0, 0 4, 10 4, 10 0, 0 0))>
>>> from shapely import Polygon
>>> polygon = Polygon([(0,0), (0, 4), (1,3), (3,4), (10, 4), (10, 0), (0, 0)])
>>> simplify_polygon_hull(polygon, 0.1, parameter_mode="vertex", is_outer=False)
<POLYGON ((0 0, 10 0, 3 4, 0 0))>
>>> simplify_polygon_hull(polygon, 0.1, parameter_mode="vertex", is_outer=True)
<POLYGON ((0 0, 0 4, 10 4, 10 0, 0 0))>
>>> simplify_polygon_hull(polygon, 0.1, parameter_mode="area", is_outer=False)
<POLYGON ((0 0, 10 0, 10 4, 3 4, 1 3, 0 0))>
>>> simplify_polygon_hull(polygon, 0.1, parameter_mode="area", is_outer=True)
<POLYGON ((0 0, 0 4, 10 4, 10 0, 0 0))>

This shall keep the example in line with the rest of the file.

Comment on lines +973 to +975
np.bool_(is_outer),
np.intc(parameter_mode),
np.double(parameter),
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the reason for casting to numpy dtypes here? None of the other functions require that.

Comment on lines +943 to +944
Larger values produce less concave results. A value of 1 produces
the convex hull; a value of 0 produces the original geometry.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is actually correct only for area mode. In the vertex mode, it is the vice versa. 0 is a convex_hull, while 1 is original geometry (as in PostGIS). That can be a bit confusing. I would recommend keeping both the same, probably following the vertex mode and PostGIS.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, it is convex hull only if is_outer=True, otherwise it is the largest triangle within the geom (I think).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants