Refactor shapely.ops to not use lgeos#1065
Refactor shapely.ops to not use lgeos#1065jorisvandenbossche merged 7 commits intoshapely:shapely-2.0from
Conversation
34acbc1 to
d2b3727
Compare
36f8f74 to
31c00ad
Compare
9f2015b to
62923e2
Compare
40800ba to
8314c0e
Compare
|
This is ready now: it fully removed all remaining usage of ctypes/lgeos from ops.py (and then remaining usage is in strtree.py) |
| The points are returned in the same order as the input geometries. | ||
| """ | ||
| seq = lgeos.methods['nearest_points'](g1._geom, g2._geom) | ||
| seq = pygeos.shortest_line(g1, g2) |
There was a problem hiding this comment.
I was about to ask about why nearest_points wasn't in PyGEOS, but then I see pygeos/pygeos#334
There was a problem hiding this comment.
Yeah, we will need to decide on whether we want to expose both versions top-level in Shapely 2.0 (but that doesn't need to be done in this PR)
shapely/ops.py
Outdated
| if tolerance: | ||
| errstr += " Try running again with default tolerance value." | ||
| raise ValueError(errstr) | ||
| raise pygeos.GEOSException(errstr) from err |
There was a problem hiding this comment.
I'm OK with exposing GEOSException, but I thinks @sgillies has preferred raising a Shapely error instead?
There was a problem hiding this comment.
For this specific case we have easy control over it (since we are catching the original GEOSException anyway to add some context to the error message), but the general issue of raising GEOSException to the user or not is a broader discussion, and some discussion is in #991
There was a problem hiding this comment.
I very much prefer to raise an error that derives from ShapelyError (https://github.com/Toblerity/Shapely/blob/master/shapely/errors.py#L4). How about a VoronoiDiagramError ?
There was a problem hiding this comment.
Note that GEOSException is "our own exception class", just like ShapelyError is (once pygeos is incorporated into shapely).
I would prefer if we can then revive the discussion in #991, which exactly deals with this topic.
I could certainly create a custom VoronoiDiagramError for this case (to get this PR merged), but it would then be adding a special case, since most other operations raise GEOSExceptions in the shapely-2.0 branch. So personally I would rather first get to conclusion for the general discussion.
There was a problem hiding this comment.
Ping here. Is it OK to keep this discussion for #991, and move forward with this PR?
I will keep the current ValueError instead of changing it to GEOSException, then this PR is just the status quo on that aspect.
Replacing direct
lgeoscalls with pygeos python functions in theshapely.opsmodule.The remaining ones to do are
polygonize,polygonize_fullandnearest_points. ForpolygonizeI already have a PR in pygeos, the others still need to be added there.