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

Precision of the WKT repr #8

Closed jorisvandenbossche closed 1 week ago

jorisvandenbossche commented 1 year ago

It might be nice to get a better float (using some rounding) representation:

In [8]: spherely.LineString([(-1, -1), (1, 1)])
Out[8]: LINESTRING (-0.9999999999999998 -1, 0.9999999999999998 1)
paleolimbot commented 1 year ago

It looks like I put this in the WKTWriter but didn't expose it: https://github.com/paleolimbot/s2geography/blob/master/src/s2geography/wkt-writer.cc#L20-L21

(The WKT writer is a shim and will be replaced by a new one that actually has dedicated tests: https://github.com/geoarrow/geoarrow-cpp/blob/main/src/geoarrow/geoarrow.h#L169-L184 )

jorisvandenbossche commented 1 year ago

GEOS started to use https://github.com/ulfjack/ryu to use a natural "shortest" printing by default (eg 0.3 instead of 0.300000011..). Might be overkill on the short term, but just noting it here.

(that's also more an issue for s2geography)

paleolimbot commented 1 year ago

Yes...also ~5x faster than snprintf (from my limited set of benchmarks)! It looks like I left a hook for it as a compile-time option ( https://github.com/paleolimbot/s2geography/blob/master/src/s2geography/wkt-writer.cc#L6-L11 ), although I'd like to replace this particular WKT writer with the one in geoarrow-cpp (which has exhaustive tests at the C level).

benbovy commented 1 week ago

@jorisvandenbossche I think we can close this now?

jorisvandenbossche commented 1 week ago

Indeed, we have an argument exposed to control the precision now, and have set the default for the repr to a lower value.