API: what should is_ccw return for non rings / polygonal input?

[jorisvandenbossche]

The is_ccw function was implemented in pygeos/pygeos#201, and there was actually already some discussion about that at the time as well (pygeos/pygeos#201 (comment)).
This function is new in 2.0.0 (there was already a Linearring.is_ccw attribute as well, but that would not be affected by this issue).

We implement is_ccw based on GEOS' GEOSCoordSeq_isCCW, which works on individual coordinate sequences (not geometries) and makes some assumptions (eg that the sequence is closed).
As a consequence, some of the additional logic to work with geometries is implementated on our side (impl in ufuncs.c).

Some aspects of the current (documented) behaviour:

• The function works for both LinearRings and LineStrings. But, for LineStrings, it is not checked that it is actually closed (while the GEOS implementation assumes this is the case)
• The function does not work on Polygons (so one would need to manually get the rings of the polygons and check those)
• The function returns False for non-linear geometries

If we compare this behaviour with for example the PostGIS equivalent ST_IsPolygonCCW:

• Also works on Polygons (in which case it checks that the inner rings have the opposite orientation). Or actually, only works for polygons (the name of the function also indicates this), and so does not work for LineString
• Returns True if the geometry has no polygonal components

Of course this function is a bit different, since it is focused on Polygons (and doesn't consider even LinearRings, but this might be because PostGIS in general tries to avoid directly exposing LinearRing, eg ST_ExteriorRing also returns a LineString instead of LinearRing).

---

Based on the above, some API design questions to consider:

• Should we continue to consider any LineString as linear input? (i.e. for which the orientation is checked) I think we could restrict ourselves to LinearRings (and polygons, see below), or at least only consider closed LineStrings.
• Should we also process Polygons? (and MultiPolygons, and GeometryCollections that contain polygons?) In the case of polygons, it would check that the exterior ring is CCW, and that all interior rings have the opposite orientation.
• In Implement is_ccw for GEOS >= 3.7.0 pygeos/pygeos#201 (comment), we already noted that this is more a 3-state problem instead of 2 binary states (True/False), since a linear geometry can be CCW, CW or the orientation can be undefined (not linear, too few points, ..). As a result of this, you can't really get "is clock-wise" as inverting "is counter-clock-wise" (is_cw = ~is_ccw(..)). For that reason, we should maybe consider adding a is_cw as well? (postgis also has both)
• Potentially the biggest change: should we default to True instead of False for non-linear inputs? (i.e. follow the postgis behaviour) This might depend on the use case what is most useful. But for example, if you would like to use is_ccw to check that all your geometries are correctly oriented (for an application that requires all rings to be CCW), you actually want True to be able to use is_ccw(input_geometries).all(), since non-linear/polygonal geometries don't have a orientation and thus are always fine (currently one would need to do is_ccw(input_geometries[np.isin(get_type_id(input_geometries), [2, 3]]).all(), assuming that polygons are handled)

Lastly, if we are going to add a force_ccw function (#1366), it would also be nice that those two are consistent with each other (for example if force_ccw would re-orient rings and polygons (and not LineStrings), ideally also is_ccw would check those types).

Since is_ccw is a new function in 2.0.0, I think we could still consider quickly tweaking the default behaviour a bit in 2.0.1

cc @caspervdw @brendan-ward

[idanmiara]

I've implemented is_ccw for any GEOS version (using a fallback), and force_ccw in #1690

[idanmiara]

@jorisvandenbossche I think that #1690 also solves most of the issues you raised in your comment above.

[brendan-ward]

Adding support for polygons seems like a good idea, and dropping support for LineStrings seems reasonable given issues of self-intersections causing problems around determining orientation.

Adding is_cw seems reasonable as well.

Instinctively, keeping False as the return value for unclosed / non-orientable / invalid geometries seems reasonable, so in this case both is_ccw and is_cw would return False. If the next step is to then reorient polygons / linear rings, this means that the user would end up passing in more geometries than actually needed to force_ccw, but that could simply clone and return geometries of types that are not reorientable (so, returning a LineString as-is regardless of whether it is closed or not). And the user could screen those out in advance based on geometry type to cut down on the overhead if they wanted.

[idanmiara]

Adding support for polygons seems like a good idea, and dropping support for LineStrings seems reasonable given issues of self-intersections causing problems around determining orientation.

Adding is_cw seems reasonable as well.

Instinctively, keeping False as the return value for unclosed / non-orientable / invalid geometries seems reasonable, so in this case both is_ccw and is_cw would return False. If the next step is to then reorient polygons / linear rings, this means that the user would end up passing in more geometries than actually needed to force_ccw, but that could simply clone and return geometries of types that are not reorientable (so, returning a LineString as-is regardless of whether it is closed or not). And the user could screen those out in advance based on geometry type to cut down on the overhead if they wanted.

Returning None for invalid/not closed - could be even a better, so no need for is_cw. Casting it to bool would also be almost fully backwards compatible.

[brendan-ward]

Returning None for invalid/not closed - could be even a better

We had discussed that in pygeos #201(comment) and ruled out None so that we could keep the data type as np.bool.

[jorisvandenbossche]

Instinctively, keeping False as the return value for unclosed / non-orientable / invalid geometries seems reasonable, so in this case both is_ccw and is_cw would return False.

The question that also comes up here, is that if we support Polygon, then what about GeometryCollections that include a Polygon?

[idanmiara]

Instinctively, keeping False as the return value for unclosed / non-orientable / invalid geometries seems reasonable, so in this case both is_ccw and is_cw would return False.

The question that also comes up here, is that if we support Polygon, then what about GeometryCollections that include a Polygon?

Why won't we?
Also, already implemented in #1690

[jorisvandenbossche]

Sorry, I should have clarified: I would find it inconsistent that if ìs_ccw(point) returns False, that is_ccw(collection[point, polygon]) can return True.
(naively, checking a collection would be checking each element like all([is_ccw(part) for part in collection.geoms], but that wouldn't hold if is_cww(point) = False)

[idanmiara]

Sorry, I should have clarified: I would find it inconsistent that if ìs_ccw(point) returns False, that is_ccw(collection[point, polygon]) can return True. (naively, checking a collection would be checking each element like all([is_ccw(part) for part in collection.geoms], but that wouldn't hold if is_cww(point) = False)

Oh, I got confused and actually referred to force_ccw.
I think that the issue could be resolved with a variant of is_ccw that returns one of the 3 options - ccw, cw, nither. maybe np.int8 and using -1 for nither.

[brendan-ward]

It seems like it should always be False for GeometryCollections as unsupported geometry types (even if they only contain polygons) for exactly this reason: the collection itself is not orientable, only its members, and with mixed member geometry types orientation is ambiguous. To check orientation and re-orient if necessary, it seems reasonable to expect the user to explode the collection into individual types before is_ccw / force_ccw then re-compose the collections. Applying geometry type specific behavior to GeometryCollections seems problematic at best (but likely just my opinion).

Otherwise, is_ccw should always return False for any GeometryCollection that contains something other than Polygon / MultiPolygon / LinearRing regardless of the orientation of those geometries within the collection (interpreted as an AND logical operator across orientation of individual geometries) - which then leads to behavior that varies based on the geometry type diversity within the collection. Best to avoid ambiguity and advertise that GeometryCollections don't support orientation checks.

[idanmiara]

To check orientation and re-orient if necessary, it seems reasonable to expect the user to explode the collection into individual types before is_ccw / force_ccw then re-compose the collections.

I think that force_ccw is well defined for any geometry type, also for points and collections
As for is_ccw your approach sounds good.