benbovy / spherely

Manipulation and analysis of geometric objects on the sphere.
https://spherely.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
119 stars 8 forks source link

Allow constructing a Polygon(..) with repeated first / last coordinate pair #42

Closed jorisvandenbossche closed 1 week ago

jorisvandenbossche commented 2 months ago

Currently we get:

In [35]: spherely.Polygon([(0, 0), (1, 1), (0, 1), (0, 0)])
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
Cell In[35], line 1
----> 1 spherely.Polygon([(0, 0), (1, 1), (0, 1), (0, 0)])

ValueError: loop is not valid: Edge 3 is degenerate (duplicate vertex)

I am used from shapely that explicitly passing the closing point (which is the same as the first point) is allowed (also because the GEOS data model actually stores this in the coordinates), but this currently fails in spherely. Also because things like WKT or GeoJSON show this, I think it is a common pattern to pass this.

We can probably detect that the last coordinate pair is the same as the first and drop it for the user.

benbovy commented 1 month ago

In #51 both closing vs. non-closing are supported, although this is fragile as it relies on strict equality of the 1st and last computed vertices (S2Point). We'll certainly need to revisit that as well as uniformize the API with #49 and #50 (i.e., add oriented and planar arguments).

jorisvandenbossche commented 1 month ago

although this is fragile as it relies on strict equality of the 1st and last computed vertices (S2Point).

I think that is fine (at least for now), AFAIK in GEOS / shapely we also rely on exact floating point equality (that can give problems in corner cases, eg after transforming coordinates, but it's not that common)

benbovy commented 1 week ago

This is now supported (#51 has been merged).