adamfranco / curvature

Find roads that are the most curvy or twisty based on Open Street Map (OSM) data.
http://roadcurvature.com/
225 stars 39 forks source link

Fix unit tests for Collector to work with Osmium #51

Closed adamfranco closed 4 years ago

adamfranco commented 7 years ago

9b0fa2add94035644fce438234a9c982cf771b5d converted from using the imposm.parser to the Osmium PBF parser. In the process, the Collector API changed and dropped the parser_class argument that had been used for wiring in test structures for unit testing.

darylmatuszak commented 4 years ago

I took a crack at this here

Will need a bit more work but would appreciate a peer review before I continue refining it.

The problems this cause seem to be limited to the tests in test_collector.py, preventing them from running due to the mentiond API change.

Since WayCollector can only parse via its parent's apply_file method, I took the mock data in the tests and used Osmium.SimpleWriter to generate files that can serve as fixtures.

Having done this, all tests except test_collector_doesnt_drop_unjoined_ways pass. Perhaps someone more familiar with the codebase can assist in determining if this failure is indeed genuine (I suspect it might be).

A few more notes:

adamfranco commented 4 years ago

Thanks for your work on this @darylmatuszak ! :-) I've made a small tweak in https://github.com/adamfranco/curvature/commit/30de14ea6ec5f3dafc3a4dd4947238f5d46a8568 to get the tests running on Python 3.5 and will take a look at the failures and give it a more full review.

adamfranco commented 4 years ago

A note to myself: I needed to upgrade osmium from 2.14.3 to 2.15.4 in order to get the fix for wrong precision being written: https://github.com/osmcode/pyosmium/issues/83

adamfranco commented 4 years ago

I took a look at test_collector_doesnt_drop_unjoined_ways and found that it was failing because the collections were getting returned in different orders than expected. The order of the collections in the result set isn't important, so I changed the test code in https://github.com/adamfranco/curvature/commit/540fc644efa4b30ca9e1a87980fd764a7b33ad95 to filter out the individual collections based on their data rather than assuming their order. Now the test will pass if both exist and look correct independent of which order they appear in.

All tests are passing now in my test-compatability branch-- getting this working again is a great improvement. I'll be away from the computer for the weekend, but 👍 on continuing this work.

darylmatuszak commented 4 years ago

Nice! Appreciate your expertise on sorting out that failure.

pyosmium is aware of the deprecation warning, and it seems to be coming from pybind11.

This is a cool project, and certainly one I will be getting a lot of mileage out of :)

As I continue to become more familiar, I hope to find further areas where I can make a contribution.

adamfranco commented 4 years ago

Thanks again for your help on this issue, @darylmatuszak! I've merged in your your work. I've been hard at work on a new in-browser map for Curvature over the past few months and haven't been giving the underlying code the love it deserves recently. Now that the new map is launched I'm hoping to find some more time for some features I've put on hold. :-)