Drop flatgeobuf dependency in geozero crate #235
Conversation
pka
force-pushed
the
drop-flatgeobuf-dep
branch
from
f535df6 to
63475db
Compare
| @@ -23,6 +19,12 @@ fn json_to_svg() -> Result<()> { | |||
| Ok(()) | |||
| } | |||
|
|
|||
| /* | |||
There was a problem hiding this comment.
If you want to keep this, can you leave a comment for us-in-the-future explaining why it's here but commented out?
geozero/tests/geozero-api.rs
Outdated
| let props = feature.properties()?; | ||
| assert_eq!(props["id"], "DNK".to_string()); | ||
| assert_eq!(props["name"], "Denmark".to_string()); | ||
| // process_properties is not public. We should have iterators! |
There was a problem hiding this comment.
Can you explain more what you mean by "we should have iterators"?
My guess is that you mean something like:
GeoJsonLineReader::next(&mut self) -> Result<Option<geojson::Feature>>
So that we can write:
while let Some(feature) = json.next()? {
...
}
Is that right? Or something else? If so, that seems reasonable.
Though, TBH, this example doesn't seem like a great fit for the geozero API anyway. In my experience, geozero is mostly concerned with processing entire datasets. This example relies mostly on things intrinsic to flatgeobuf, including some API's added to FGB as helpers for implementing geozero APIs, but are not actually geozero APIs themselves.
There was a problem hiding this comment.
Exactly. We should have a common feature iterator for GeozeroDatasource.
There was a problem hiding this comment.
I'd really love to have a feature iterator. I've thought briefly about it before but I got stuck because I don't know how to combine the push-based nature of the GeomProcessor with the pull-based nature of the iterators
| @@ -43,7 +43,6 @@ hex = "0.4" | |||
| kdbush = "0.2" | |||
| log = "0.4.19" | |||
| lyon = "1.0.1" | |||
| polylabel = "2.5" | |||
There was a problem hiding this comment.
I'm guessing you've removed this example because it uses FGB. We could convert it to use geojson, but I'm guessing you've judged it to be not worth the upkeep?
That seems fine, just making sure I understand.
There was a problem hiding this comment.
Exactly. This example should go either into the flatgeobuf or the polylabel crate. I tried to use geojson, but I had the same problem with the missing feature iterator.
pka
force-pushed
the
drop-flatgeobuf-dep
branch
2 times, most recently
from
3477741 to
f5ec4e0
Compare
|
Everything's looking good, but I'll let you take care of sequencing your various merges @pka. |
pka
force-pushed
the
drop-flatgeobuf-dep
branch
from
f5ec4e0 to
463f721
Compare
Solves flatgeobuf/flatgeobuf#283