georust / geozero

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

[WIP] Geozero 0.7 #6

Closed pka closed 3 years ago

pka commented 3 years ago

Merges geozero-core into geozero crate

michaelkirk commented 3 years ago

I see this is still a Draft, and so presumably you're still making changes, but I did take a quick look at the current state.

When I first worked with the geozero crate, I was a little confused about the distinction between geozero/geozero-core, I think combining them makes sense to get rid of that confusion.

I notice there is still a core feature, but I don't understand why. Is core just to make the transition easier, or do you think it's useful long term?

Instead, I'd assume people would opt into the geo-types, geojson, etc. features separately as they required them.

I get why you might want some of these to be enabled by default, e.g. default = ["geo-types", "geojson", "scroll", "bytes"], but don't understand why the core "meta" feature.

michaelkirk commented 3 years ago

Also, LMK when this PR is at a stable point and I can try it out.

pka commented 3 years ago

Thanks for looking at the PR @michaelkirk!

The split into two crates was inspired by geo-types/geo and others, with the idea that geozero should be a minimal dependency for implementations like flatgeobuf and geozero-core an additional dependency for converting to other formats. After starting to implement traits à la ToGeo for geo_types::Geometry within geozero-core, I realized that we need base traits and their implementation for external types in one crate. That's the reason for this merge, since I wanted to avoid maintainingwith-geozero features for all known geometry crates.

The core feature flag is needed to still provide a minimal dependency for external implemenations like flatgeobuf. But you're right, it's an option to split the core feature into geo-types, geojson, wkb, wkt, svg etc. But I think, this can be also done in a future version.

pka commented 3 years ago

PR is now ready for testing/reviewing. The library docs need more work, but the README is up-to-date.

Started to split the core feature into individual format features in 962e72a, but stopped for the moment, since I don't know, how to enable optional features, which are used in the tests of other formats.

pka commented 3 years ago

I (mostly) cleaned up the commits and merged them into master. Testing/comments still welcome! Will reorganize formats and finish individual feature flags next.