Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Support deserializing integer geometries #102

Closed
michaelkirk opened this issue May 17, 2022 · 2 comments · Fixed by #103
Closed

Support deserializing integer geometries #102

michaelkirk opened this issue May 17, 2022 · 2 comments · Fixed by #103

Comments

@michaelkirk
Copy link
Member

michaelkirk commented May 17, 2022

#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
Copy link
Contributor

rmanoka commented May 18, 2022

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
Copy link
Member

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.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants