-
Notifications
You must be signed in to change notification settings - Fork 92
Conversation
Hi, @mvexel I prefer to put the requirements on setup.py, see https://github.com/willemarcel/overpass-api-python-wrapper/blob/master/setup.py It has the requirements to use and to test the library. You can install the library to develop and test with
and run the tests just executing |
Sorry, now I saw the setup.py is correct on this PR. So just try to test with the instructions above. I think everything is OK. |
Hey @willemarcel - this is what I get using
|
and with
|
also I think the tests need to run faster (collect less data from Overpass perhaps) |
I think I found it. I did change the interface of the CLI application. |
Hmm, tests pass locally on my machine now, but the last CLI test still fails on travis. Not sure what's going on. @willemarcel any ideas? |
…rect output to a file if wanted, more flexible) and fixing tests
I fixed the tests. I also couldn't resist simplifying the application a bit and getting rid of the output file part. You can just redirect the output to a file yourself as the CLI user, making it more flexible (for piping etc). This all happened in 99d390d If you're OK with this I'll merge. |
Just took a look at all the changes, looks good! Are there any plans of a joined development @mvexel and @willemarcel to prevent future divergence of the project? |
@itiboi I think after this merge we should consolidate development here if @willemarcel agrees that is the way to go. This project has really great potential and I think we have a group of good devs here. I propose we cut releases and publish to PyPi from this master repo from now on. I will commit some more time to the project to address issues and coordinate releases. Sounds like a plan? |
Yes, I agree! |
Great! |
Merge Wille's fork and improvements
Excellent. Merged! |
I have made an effort to cleanly merge in @willemarcel's fork, as he has been publishing independently on PyPi. I would like to consolidate the PyPi releases on the master repo.
Wille introduced a CLI application that I am not very familiar with. The tests fail using
nosetests
. I am not sure how to fix it.The main change I made is in how you define the response format. Instead of the
asGeoJSON
boolean parameter, I introduced aresponseformat
parameter that allows the valuesxml
for OSM XML,json
for plain JSON as returned by Overpass, andgeojson
for GeoJSON output.GeoJSON response is still the default so this should not break anything.
The only breaking change is that Wille's CLI application now takes those same values for its
format
parameter.Please review and see if we can merge this!