Outsource OpenStreetMap data dump access to osm-flex

CLIMADA-project / climada_petals

See https://github.com/CLIMADA-project/climada_python first

GNU General Public License v3.0

22 stars 13 forks source link

Outsource OpenStreetMap data dump access to osm-flex #103

Closed Evelyn-M closed 7 months ago

Evelyn-M commented 7 months ago

Changes proposed in this PR:

Make osm-flex a dependency, and re-introduce overpy as dependency (else accessing overpass API fails)
Delete all CLIMADA-internal code which downloaded, clipped and parsed from osm.pbf files
Update openstreetmap tutorial and tests

This PR fixes #

PR Author Checklist

[x] Read the Contribution Guide
[x] Correct target branch selected (if unsure, select develop)
[x] Descriptive pull request title added
[x] Source branch up-to-date with target branch
[x] Documentation updated
[x] Tests updated
[x] Tests passing
[x] No new linter issues
[x] Changelog updated

PR Reviewer Checklist

[x] Read the Contribution Guide
[x] CLIMADA Reviewer Checklist passed
[x] Tests passing
[x] No new linter issues

Evelyn-M commented 7 months ago

Important: Due to my lack of expertise about defining classes (i.e. OSMApiQuery), I suggest that a second reviewer has a look at the files

climada_petals/entity/exposures/osm_dataloader.py

climada_petals/entity/exposures/test/test_openstreetmap.py I looked through it, but didn't think through all the functionalities.

Since those functions were not modified in this pull requested, and were reviewed and tested before, this is fine :)

Evelyn-M commented 7 months ago

Thanks for the feedback @leonie-villiger and @peanutfun . I implement all your suggestions; tests & tutorials are updated and passing, hence merging :)

peanutfun commented 7 months ago

@Evelyn-M What's the rush? Several of my comments have not been addressed, and there is quite a number of linter issues one could still resolve, at least two of which have elevated severity. @leonie-villiger and I both requested changes which means that we have to have another round that approves the new version. If you need to hit a deadline, please say so earlier.

Evelyn-M commented 7 months ago

Sorry, there were a few hidden comments that got lost! Luckily only 2 or 3, and they're anyways part of the APIQuery part, which was actually not intended to be changed as part of this pull request (I still made quite a few changes upon your suggestions, but feel free to open a separate PR for further amendements. They're all good points, so I'll copy them over to the respective GitHub issue.) Leonie did re-review and approve, and I have no work-time left on this (sorry for the rush this time, was a pragmatic decision :) ).