bennylope / pygeocodio

:globe_with_meridians: A Python wrapper for the Geocodio geolocation service API
BSD 3-Clause "New" or "Revised" License
94 stars 21 forks source link

Updated batch_geocode for dictionaries #42

Closed liufran1 closed 4 years ago

liufran1 commented 4 years ago

This addresses #27

This is admittedly a very blunt approach to the problem: LocationCollectionDict is a version of LocationCollection that inherits from dictionary instead of list.

The batch geocoder endpoint for geocod.io accepts a JSON object. Currently batch_geocode only accepts lists of addresses. When a python dictionary of keys and addresses is passed to batch_geocode, the geocod.io interprets it as a JSON object and returns a JSON object in its response. The results field is now a JSON object rather than an array, which python interprets as a dictionary.

LocationCollection expects a list from response.json()["results"] and understandably breaks. If the response is instead a dictionary, it gets rerouted to LocationCollectionDict to create a comparable dictionary object. LocationCollectionDict changes the list methods in LocationCollection to dictionary methods

liufran1 commented 4 years ago

Thanks for the feedback! Added the suggested changes above. To note - reverse geocoding functionality hasn't been touched although it should be updated accordingly if you want to include these changes.

Unix-Code commented 4 years ago

Thanks for the feedback! Added the suggested changes above. To note - reverse geocoding functionality hasn't been touched although it should be updated accordingly if you want to include these changes.

I originally mentioned changing it to work similarly for reverse geocoding, but a rough scan of the Geocod.io API docs shows that batch reverse geocoding doesn't support a dict of reference keys to coordinate pairs. So it should be alright to leave it unchanged, unless I missed something there...

liufran1 commented 4 years ago

I originally mentioned changing it to work similarly for reverse geocoding, but a rough scan of the Geocod.io API docs shows that batch reverse geocoding doesn't support a dict of reference keys to coordinate pairs. So it should be alright to leave it unchanged, unless I missed something there...

Fair point