georust / geozero

Zero-Copy reading and writing of geospatial data.
Apache License 2.0
321 stars 30 forks source link

Move geozero-shp into geozero? #187

Open kylebarron opened 5 months ago

kylebarron commented 5 months ago

geozero-shp is out of date; it depends on an old version of geozero and therefore it seems can't be used with geozero 0.11. Is there a reason why it exists as a separate crate? Can it be moved into the main geozero crate with a feature flag? Otherwise it seems necessary to publish a new version of geozero-shp whenever geozero releases a new version

michaelkirk commented 5 months ago

gdal integration leverages the gdal crate, geojson leverages the geojson crate (etc.). But for shapefiles, all the format parsing logic lives right in the geozero-shp crate, so I'm not sure about merging the geozero-shp crate into geozero wholesale.

Maybe we could improve the situation by re-slicing the crate boundaries so that the geozero integration (and only the integration) lives within geozero, while the geozero-agnostic shapefile parsing logic lives in an external shapefile crate. @pka - do you recall why you forked shapefile-rs into geozero-shp rather than adding a geozero integration that leveraged the existing shapefile-rs crate?

pka commented 4 months ago

@pka - do you recall why you forked shapefile-rs into geozero-shp rather than adding a geozero integration that leveraged the existing shapefile-rs crate?

I can't remember the reasons, sorry. Maybe the goal was to have an example of a separate geozero impl. Or it was so flaky, that I didn't wanted to include it in the main crate.

So to answer the question - I'm fine with moving gezero-shp into the geozero crate.

kylebarron commented 4 months ago

In the medium term I do want to get shapefile working and am happy to make some PRs, but probably not before the next 0.12 release.