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

Broken empty linestring repr with last s2geography dev version #67

Closed benbovy closed 1 week ago

benbovy commented 1 week ago

Looks like the CI failing tests are caused by the changes in https://github.com/paleolimbot/s2geography/pull/37, which now raises an exception for empty S2Polyline. @jorisvandenbossche do you think we should fix that in s2geography or work around it in spherely to support empty linestrings?

jorisvandenbossche commented 1 week ago

We should fix that there if needed.

I think I assumed that an empty Linestring would have no polyline, not an empty polyline? But that assumption might have been wrong?

benbovy commented 1 week ago

Yeah the creation functions added in #51 currently add one empty polyline, but it is probably cleaner indeed to implement the empty case as no polyline here (there might actually be another bug since actually Geography.m_empty is set to true when there is no polyline rather than when there is an empty polyline).

jorisvandenbossche commented 1 week ago

In any case, "LINESTRING EMPTY" is being tested on the s2geography side, so I assume the creation methods there also create a PolylineGeography with an empty vector

benbovy commented 1 week ago

Yes we should fix this in the Spherely creation functions.