Replace homegrown converter with osm2geojson

[dericke]

Replaces the built-in method to convert Overpass JSON to geojson with the osm2geojson library. The motivation is to reduce the scope of this library and offload some maintenance burden.

Important differences:

• osm2geojson uses 7-digit coordinate precision, same as the Overpass JSON. This lib has been producing 6-digit geojson up until now
• osm2geojson nests the id attribute under properties, this lib has been putting id as a top-level attribute

Setting as a draft PR until:

• [ ] it is decided either that these differences are acceptable, or either osm2geojson or this PR is modified to produce identical output
• [x] it is confirmed that this will not regress recent PRs such as #130

Closes #135 and possibly some others?

[mvexel]

Hey cool! Sorry it took me so long to respond. Since this would be a breaking change, with the id moving to properties, this would mean a 1.0 release. I would like to shore up tests before we do that, but in general I like not relying on bespoke code when a well-established library is available.

What work do you think needs to be done to the unit tests? Last I checked test coverage is still pretty poor...

[dericke]

Testing is not my greatest strength, but I think #141 would be a good start in improving the tests. I'll try to do some more comprehensive analysis to see what can be improved beyond that.

[mvexel]

@dericke what is left to do before we can merge this? Should there be a 1.x branch since this would be a breaking API change? @russbiggs can you comment on whether this will break #130 ?

[dericke]

I was just waiting until the PR related to tests was merged, which I see you've done (thanks!), and for feedback on the API change. I don't feel confident on judging whether the API change is significant enough to warrant a major version increment or not.

[russbiggs]

@mvexel it looks like this would remove all the changes I made in #130 since it removes the whole _as_geojson method. I'm not sure if osm2geojson would do the same as what my PR does.

[dericke]

It does appear that this PR currently regresses #130 . I'll look into doing a PR against osm2geojson to fix this.

[dericke]

The PR for osm2geojson was accepted, so when using version >= 0.1.30 of that library, #130 is not regressed.

[mvexel]

Hey, see my comment here: https://github.com/mvexel/overpass-api-python-wrapper/issues/135#issuecomment-1075451525 Do you agree it makes sense to adopt __geo_interface__? /cc @sgillies

[dericke]

I like the idea of supporting __geo_interface__, but wouldn't that still require a converter—provided either by this lib or an external dependency—to go from overpass json output to __geo_interface__-compatible output?

[mvexel]

I like the idea of supporting __geo_interface__, but wouldn't that still require a converter—provided either by this lib or an external dependency—to go from overpass json output to __geo_interface__-compatible output?

Yes, it would. But then any library that adopts __geo_interface__ would be able to handle the output.