search

what3words / w3w-python-wrapper

Python wrapper for the what3words API
https://docs.what3words.com/api/v3/
MIT License
78 stars 14 forks source link

Svartalf master minus conflicts #2

Closed ghost closed 9 years ago

ghost commented 9 years ago

I was wanting to see a setup.py for this myself, but @svartalf's PR had merge conflicts that he hadn't handled. I resolved those, and merged with my fork, which was even with what3words/w3w-python-wrapper:master beforehand. Merge should be clean now.

Also, note that unlike @svartalf, I do have an API key. 😉 I haven't written any formal unit or integration tests, but note that from an acceptance test perspective, I am now using this version (setup.py run via pip + git) in my https://github.com/yagermadden/whereami app, and it works just as well as the original master.

ghost commented 9 years ago

Also also, please note that this changes the name of the class to What3Words (capital W's!) along with lowercase-ing and shortening the names of the methods. I tend to agree with @svartalf that it's more Pythonic that way, but I notice that w3w branding prefers a lowercase name for "what3words". My conclusion would be that Python and PEP8 usage should be upheld here, but obviously you guys could merge and then change afterward if you feel differently.

In my latest commit I did update the README to match the changes. It is worth underlining that these are breaking changes. It's true my code using the wrapper works with the installed module, but I had to make some (small) updates.

vicchi commented 9 years ago

This is excellent stuff; thank you! We're taking a look at restructuring and refactoring all the API wrapper repos in line with best practice for their respective ecosystems and this is totally aligned with that. I'll get this merged in, tested and tagged as soon as I have the bandwidth (in other words, probably not today but hopefully by the end of this week). Thank you once again.

ghost commented 9 years ago

Thanks very much, @vicchi! Also, please see https://github.com/what3words/w3w-python-wrapper/pull/3

ghost commented 9 years ago

Arg - Sorry, I forgot I made this PR from my master branch. Now, since I merged that in my fork, it's been added to this PR, which I intended to leave without the Requests lib change. I'm going to close this and let you use #3 if you like it. If you'd like me to recreate this with just my adjustments to @svartalf 's changes to resolve the merge conflicts, let me know and I can make a new PR from a branch for that. Again, my apologies for the mess.