compute_voronoi should return a Vec<GGeom>

[GuillaumeGomez]

There is no point into making a conversion inside geos, it should be done alongside or outside but not inside the compute_voronoi function.

[antoine-de]

hum if you don't want  geos objects, you can use ggeom::voronoi, compute_voronoi was meant as a geo wrapper around it (maybe the naming needs to be changed if that's not clear).

What don't you want this ?

[GuillaumeGomez]

It's supposed to bind geos and only that. If you want a conversion, it has to be done outside of this crate. Otherwise it'd become just a huge patchwork.

[antoine-de]

so you want to also move from_geos.rs ?

lots of crate in the geo ecosystem provide a way to convert from/to geo object (geojson, wkt, ...)

I'm not sure to see the worth of moving it to a separate crate, but if you prefer and if the usage of this crate from geo object is not made more complex I'm ok with it

[GuillaumeGomez]

To be more clear: I'm feeling uneasy about the whole geo-types usage in a crate supposed to only provide a Rust interface to a C(++) library.

[GuillaumeGomez]

Also: it's pretty common to have the C functions declaration and C library linking done in a separate crate and to have wrapper inside its own crate. Basically, having a ffi crate and a "rust" one. Don't know if that makes sense in here?

[mthh]

I have to admit that in my opinion the code contained in voronoi.rs does not really belong here (although it is probably very useful) in the sense that it takes geo-types as input and returns geo-types (and the voronoi method in ffi.rs is already doing the job to make voronois from / to GGeom).

I may not be as uncompromising about conversion (as in from_geo.rs and to_geo.rs) if it turns out geo/geo-types to be the reference crate for manipulating geometry in Rust or the one that serves as an interoperability layer between crates that handle 2d geometries. And I guess if needed we could easily define some "geo-conversion" optional feature and move that conversion code to it ?

Regarding the fact of having two separate crates (ffi + higher level bindings), I think you're right and it should probably be done !

[antoine-de]

for the moment the ffi is contained in one module, the rest of the crate is for conversion with the geo-types

I do agree with the fact that voronoi.rs is strange there, but I do believe we need a layer to hide the complexity of calling geos from geo-types (it might be done in a better way than free functions though). If you want to move this in another crate it's fine for me.

[urschrei]

(I haven't used geos, so this is only context, not opinion) having a -sys crate is nice, because as you say, it allows you to keep all the complexity in one place – it's what we do for PROJ.4 (https://github.com/georust/proj-sys for the wrapper and https://github.com/georust/proj for the more familiar Rust interface) and it's worked very well, IMO.