search

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

WIP: Impl/from geo types #88

Closed categulario closed 2 years ago

categulario commented 2 years ago

addresses #73 and #48.

This adds impls of From for every type and the Geometry enum in geo_types. It makes it unnecessary to bring to scope the ToWkt trait which gets deleted because it was not being used actually. Of course this is a breaking change.

Additionally I added some docs on the most relevant cases for this crate: parsing a wkt and getting a geometry and having a geometry and getting its WKT string. This is what actually motivates most of the PRs I've been doing, I just wanted to do that .-.

Some opinions

What if Wkt was instead a trait itself, implemented for every type in geo_types that provided a single method: wkt(&self) -> String?

being georust an environment for building geo applications it makes sense that things are integrated to work well within the environment, and maybe providing some LineString etc. traits which, if implemented on foreign types would give automatically a Wkt impl that allows to get its WKT string. And perhaps a WktGeometry enum with inner types refering to those in the geo_types crate just for the purposes of having something to parse a string into. Although it probably makes more sense to just have FromStr impls for the geo_types themselves...

Just an idea, I think it would simplify a lot how work with this crate happens.

This actually requires #86 for the docs to pass the tests

categulario commented 2 years ago

it also addresses #55

michaelkirk commented 2 years ago

I'm curious what you though of my proposal in #89 @categulario

categulario commented 2 years ago

With Display implemented for Wkt<T> I don't tink it is necessary because people using ToWkt are already getting a Wkt out of it, which can be to_string()ed

categulario commented 2 years ago

Just pushed some commits and rebased main.

ToWkt is restored but deprecated. The doc tests are fixed.

categulario commented 2 years ago

Closing this since it no longer reflects my ideas, mostly laid out in #73