bennylope/pygeocodio

Issue with Batch Reverse Geocoding

snake-plissken opened this issue · 2 comments

GeocodioClient.reverse() needs to be adjusted so that batch reverse geocoding can work properly. I'm using Windows-7 32-bit.

>>> from geocodio_old import GeocodioClient
>>> client = GeocodioClient(api_key)
>>> address_list = ['40.8623,-73.9239', '40.8433,-73.9421']
>>> data = client.reverse(address_list)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "C:\Python27\lib\geocodio_old\client.py", line 21, in wrapper
  return f(*args, **kwargs)
File "C:\Python27\lib\geocodio_old\client.py", line 201, in reverse
  return self.reverse_point(args[0][0], args[0][1], fields)
File "C:\Python27\lib\geocodio_old\client.py", line 21, in wrapper
  return f(*args, **kwargs)
File "C:\Python27\lib\geocodio_old\client.py", line 176, in reverse_point
  return error_response(response)
File "C:\Python27\lib\geocodio_old\client.py", line 34, in error_response
  raise GeocodioDataError(response.json()['error'])
geocodio_old.exceptions.GeocodioDataError: Could not parse coordinate

It looks like the problem is in GeocodioClient.reverse():

def reverse(self, *args, **kwargs):

    fields = kwargs.pop("fields", [])
    if len(args) == 1:
        return self.reverse_point(args[0][0], args[0][1], fields)
    return self.batch_reverse(args, fields)

The if statement resolves to true unless more than one argument is passed. However, passing more than 1 argument will throw an error since GeocodioClient.batch_reverse() only takes 1 argument. I have submitted a pull request for this fix.

Also, as per the documentation that has an example where a list of lat-long tuples is passed to GeocodioClient.reverse(), this also throws an error:

>>> list_tuples = [(40.8555,-73.8843), (40.85075,-73.8945)]
>>> client = GeocodioClient(api_key)
>>> data = client.reverse(list_tuples)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "C:\Python27\lib\geocodio_old\client.py", line 21, in wrapper
  return f(*args, **kwargs)
File "C:\Python27\lib\geocodio_old\client.py", line 201, in reverse
  return self.reverse_point(args[0][0], args[0][1], fields)
File "C:\Python27\lib\geocodio_old\client.py", line 21, in wrapper
  return f(*args, **kwargs)
File "C:\Python27\lib\geocodio_old\client.py", line 176, in reverse_point
  return error_response(response)
File "C:\Python27\lib\geocodio_old\client.py", line 34, in error_response
  raise GeocodioDataError(response.json()['error'])
geocodio_old.exceptions.GeocodioDataError: Could not parse coordinate

There were two code issues and a documentation issue. The patch in #5 solved one of the code issues, that is the treatment of points. Using *args seems more elegant, but using a list is certainly more straightforward. I basically used the code from #5 and added you as a contributor. The other issue - the main issue - was that the points were being serialized properly. That's why Geocodio sent back the Could not parse coordinate error. That's also solved in 12668bf. As for the documentation, the docs in the repo here look correct, however the Geocodio docs on this are incorrect. once the pull request is accepted this should all be up to date.

Thank you for persisting with this.