use geo-traits for Kernel and Winding

[JosiahParry]

• I agree to follow the project's code of conduct.
• I added an entry to CHANGES.md if knowledge of this change could be valuable to users.

---

This PR is definitely ambitious. I dream of a future where the algorithms in geo are agnostic to the type of geometry that it is passed into. In my case, I need to perform winding checks to Esri polygons and would like to be able to do this without converting them directly into geo_types::Polygon/MultiPolygon.

In main, the Winding trait is only implemented for LineString type—but, if geo were to add geo-traits as a dependency and use LineStringTrait as an alternative, this can be possible.

I'm not sure if there is consensus in having geo support geo-traits, but if so, consider me happy to help!

[JosiahParry]

the .unwrap()s make me sweat but I didn't do it first!

[JosiahParry]

I think one great place to start after this would be implementing LinesIter for LineStringTrait instead of LineString as that would make it quite straight forward to adapt a bunch of other algorithms such as area

[kylebarron]

It looks like this is essentially backwards-compatible? The trait definition changes, of course, but anyone who was using this on geo::Coord doesn't have to change their code?

[JosiahParry]

@kylebarron as far as I can tell, this should be 100% backwards compatible. I did my best to ensure that, while the traits are changed a smidgen, they’re compatible with the existing implementation.

The key challenge was changing the Kernel trait to use CoordTrait with T CoordNum instead of Coord

I think there are a few of these other traits that are super important throughout the crate—like LinesIter for example—that, if changed to use geo-traits, would make the rest follow suit fairly simply

[michaelkirk]

Even if this is a backwards compatible change, I think we should be clear about where we're heading before we adopt geo-traits into the geo public API.

I'd assume the idea is ultimately to phrase most (all?) of our public API's in terms of geo-traits inputs. So the implications of merging this are actually pretty big in my mind, and I think we should be confident that we can succeed before we start merging this admittedly reasonable integration into the main branch.

Also, just to make sure I'm understanding, even if all of our algorithms are updated to accept geo-traits as input, any constructive operation (those that return new geometries, like Buffer, AffineTransform) would still return geo-types. Right?

Previous discussion showed that some algorithms went easily, while others were not so great, and caused a lot of boilerplate code. IIRC Area (or maybe anything implemented on individual geometries) required a lot of boilerplate to tie everything together. Would we still want to proceed if geo-traits-as-input only makes sense for some algorithms?

[JosiahParry]

I think we should be clear about where we're heading before we adopt geo-traits into the geo public API.

Yes, I agree! From my less-than-active perspective, I would like for the API of geo to accept geo-traits as inputs for all algorithms. All algorithms that generate geometries ought to return geo-types as there is no way, using traits alone (unless there's some magic with impl From<impl YourGeoTraitHere>).

From a quick find and search there are 100 pub traits that are used throughout geo. It would require a lot of elbow grease, but I don't think it would be impossible.

[michaelkirk]

I also want to clarify: I don't think the bar should be that we migrate every single API to geo-traits before we accept any geo-traits code in geo. But I think we should be confident that we've captured the patterns by migrating a handful of differently styled algorithms and laid out a plan of attack.

[JosiahParry]

Or—perhaps it is impossible in some cases?

I'm taking a look at Centroid as it is "fairly simple"—but it leads me to wonder if rust's inheritance may not permit something like this:

trait MyTrait {
    fn fx() -> f64;
}

impl<T> MyTrait for T
where
    T: LineTrait,
{
    fn fx() -> f64 {
        todo!()
    }
}

impl<T> MyTrait for T
where
    T: LineStringTrait,
{
    fn fx() -> f64 {
        todo!()
    }
}

[kylebarron]

Or—perhaps it is impossible in some cases?

It's not possible as you've shown because Rust doesn't have specialization. Some options:

• Different algorithm trait names for impls on different geo-traits types. Not great.
• Implement main public algorithm trait on GeometryTrait instead of each core geometry trait type. (Since as of geo-traits 0.3, all traits also implement GeometryTrait). At the cost of an enum lookup, this might be the best option for things like Area that return a number and are implemented for all geometry types. But this wouldn't work for algorithm traits that are only implemented for some geometry trait types, and it wouldn't work for stuff like affine transforms where you want to return a typed struct of the same type.