georust / wkt

Rust read/write support for well-known text (WKT)
https://crates.io/crates/wkt
Apache License 2.0
50 stars 25 forks source link

Support deserializing integer geometries #102

Closed michaelkirk closed 2 years ago

michaelkirk commented 2 years ago

101 added support for serializing integer geometries to WKT, but I hit an issue while implementing deserialization for integer types.

My WIP is here: https://github.com/georust/wkt/tree/mkirk/try-from-non-floats

The issue is that numbers with decimal points will not parse as integers. For example i32::from_str("1.9") will Err.

Arguably that's expected. But I can also imagine people wanting us to "just make it work" either by rounding or truncating the portion past the decimal.

In any case, the current error message is confusing:

value: InvalidWKT("Missing closing parenthesis for type")

I'd at least expect something like "failed to parse numeric type"

I've punted on this now by simply not adding support for parsing non-float types yet.

rmanoka commented 2 years ago

Err-ing if the scalar can't be parsed feels like the right thing to do; agree that the error message could be more helpful. I don't think there's a huge use-case for this, so we could wait and hear more thoughts. Thanks for all the effort!

urschrei commented 2 years ago

I agree that Err is the way to go here – with a more informative error, its resolution can be left up to the library user.