Geometry type system design propersal

[sunng87]

I'm not native English speaker, if you have any question, just point it out and I'll try my best to make it clear.

After we reached agreement to design the type system against some open standard, it's time to go further.

My idea is to make an enum Geometry as the entry point of our type system. It will be like the Json enumeration in rustc-serialize, because in Rust, enum is a good way for polymorphism. When we are parsing geometry objects from Geojson or WKT, we might have no idea about the concrete type of them so we can use Geometry. Concretely, we will have a FromWkt trait and a ToWkt trait for I/O on Geometry without knowing the exact type. We can use pattern matching then to deal with different types. So other operations can also be impled on Geometry, like DE-9IM. It computes relationship between two objects (Point-LineString, Point-Point, LineString-Polygon or whatever).

The value of enum will be the structs for different types. We still need concrete types because some operations or attributes are unique to some type.

enum Geometry {
  Point (Point),
  LineString (LineString),
  Polygon (Polygon),

  ...
}

We many have some functions like as_point(&self) -> Option<Point> to convert it from Geometry to Point.

I will be working on a simple implementation for this. Any question is welcomed.

[frewsxcv]

This all sounds good to me. In regards to:

Concretely, we will have a FromWkt trait and a ToWkt trait for I/O on Geometry without knowing the exact type.

Instead of this, what about having FromGeo and ToGeo on all the readers/writers (e.g. WKT, GeoJSON, etc). That way rust-geo doesn't need to know about all of the readers/writers. Pros/cons/thoughts?

[frewsxcv]

I created a couple extremely basic dependency diagrams to illustrate what I'm talking about.

---

What @sunng87 proposed

---

What I counterproposed

[frewsxcv]

@georust/admins This is a pretty significant architecture/design decision. Would be great to get everyone's feedback

[groteworld]

Thanks @sunng87!

I think the To/FromGeo option is better as well, easily allows other markups to interface with rust-geo

[sunng87]

ToGeo and FromGeo works for me, too.

[mgax]

To me, impl FromX for Y means "parse X and give me Y", and impl ToX for Y means "serialize Y as X", so FromWkt and ToWkt make sense.

[sunng87]

@mgax I thought FromWkt and ToWkt is more straightforward at the beginning. But in Rust you cannot implement traits on a struct outside the crate it is defined. That means, if we use FromWkt and ToWkt, we will have to implement them within the geo crate. Also there's no way to add something like FromKml in the 3rd part crate. That's the most significant drawback of From/ToFmt style API.

[mgax]

in Rust you cannot implement traits on a struct outside the crate it is defined

You can also implement them in the crate where the trait is defined. So we can implement FromKml in the KML crate, etc.

[frewsxcv]

Right now, I don't feel strongly one way or the other, but the biggest reason I don't like ToWkt/FromWkt is it implies that rust-geo will have to implement every single one of these traits for every single file format we want to support. That means that if someone wants to use rust-geo, they'd be pulling all of these file format libraries they might not even be using.

[frewsxcv]

"To me, impl FromX for Y means "parse X and give me Y", and impl ToX for Y means "serialize Y as X", so FromWkt and ToWkt make sense."

Interesting, I never thought about the naming scheme in that way before. I always just thought

• impl FromGeo for Wkt: Construct a Wkt struct from a Geo struct
• impl ToGeo for Wkt: Construct a Geo struct from a Wkt struct

[sunng87]

@mgax cool, then I will back FromWkt style naming.

@frewsxcv ToWkt / FromWkt will be defined in rust-wkt crate, that's a feature as @mgax said. rust-geo won't depend on rust-wkt.

[frewsxcv]

Oh I think I understand now. Sounds good to me

[sunng87]

Just found we have done pretty much about types in geojson. I will pick some types from geojson and find a boundary between this base type system and geojson-specific things.

[mgax]

I've started to prototype the conversion in rust-gdal:

• converting Rust struct to GDAL using a ToGdal trait
• converting back to native - a method on the GDAL geometry; I haven't figured out how to write it as a trait on the native type.

[frewsxcv]

I've been busy the past few days, but I'll start doing the same with rust-wkt now that I have some free time

[frewsxcv]

I've completed impl ToWkt for geo::Geometry. You can see the implementation here. It's a little messy right now, but it'll clean up over time.

Last time we spoke, this was the model we agreed upon:

I was thinking about it, and I came up with a counter proposal that doesn't use from_*:

The nice thing about having ToWkt and ToGeo is it follows the (unfinished) rust guidelines:

"When in doubt, prefer to/as/into to from, because they are more ergonomic to use (and can be chained with other methods)." https://aturon.github.io/style/naming/conversions.html

and

Conversions prefixed to_, on the other hand, typically stay at the same level of abstraction but do some work to change one representation into another. https://aturon.github.io/style/naming/conversions.html

Thoughts?

[mgax]

Yup, I think we can use to_wkt and to_geo, it makes sense.

[sunng87]

For Wkt, do you think it's OK to use geo type directly ?

[frewsxcv]

@sunng87 I was thinking about that this weekend. I would love for all the readers/writers (e.g. GeoJSON, Wkt, etc) to all use the Geo::Geometry types internally, but unfortunately, their geometries are all slightly different. Wkt has items like MULTICURVE, TRIANGLE, and TIN. GeoJSON has items like Feature and FeatureCollection. If this doesn't answer your question, let me know.

[mgax]

The only significant difference is the basic point type, in rust-geo it's an x,y pair, in rust-geojson it's an arbitrary-length vector. Maybe we should pick one and then we can use the same base types.

[frewsxcv]

An RFC merged recently that's relevant

https://github.com/rust-lang/rfcs/blob/master/text/0529-conversion-traits.md