Support reverse_geocode by place_id #110
Conversation
If reverse_geocode is passed a string that does not begin with a digit or a + or - sign, assume it's a place_id rather than a latlng coordinate and treat appropriately.
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
|
I signed it! |
|
CLAs look good, thanks! |
Python3 lacks 'unicode' type. Need different approach to catch unicode strings in Python2.
|
Ick, proposed code failed on python3 due to lack of 'unicode' string type. New version in python3 only; works on python2, but fails for unicode-encoded place_ids in that case. |
| @@ -90,7 +90,10 @@ def reverse_geocode(client, latlng, result_type=None, location_type=None, | |||
| :rtype: list of reverse geocoding results. | |||
| """ | |||
|
|
|||
| params = {"latlng": convert.latlng(latlng)} | |||
| if (type(latlng) == str) & (latlng[0] not in ("-+0123456789")): | |||
There was a problem hiding this comment.
Few problems:
- As you pointed out, the type check fails. How about
convert.is_string(latlng)? &should beand- Looks like Place IDs contain alphanumeric chars and dashes, so there's a chance
latlng[0] not in ("-+0123456789")would fail if the first character in a place ID was a digit. Since the string version of a latlng would contain a comma and a Place ID wouldn't, how about checking for that?
Use convert.is_string to do 2/3-safe string detection If string, test for comma; if no comma, then place_id
|
Implemented suggested changes from @stephenmcd. Thanks and hope this is good to go. |
| @@ -90,7 +90,10 @@ def reverse_geocode(client, latlng, result_type=None, location_type=None, | |||
| :rtype: list of reverse geocoding results. | |||
| """ | |||
|
|
|||
| params = {"latlng": convert.latlng(latlng)} | |||
| if (convert.is_string(latlng) and (',' not in latlng)): | |||
There was a problem hiding this comment.
Almost there - most of those parentheses aren't necessary, so just:
if convert.is_string(latlng) and ',' not in latlng:
Could probably also use a comment mentioning why we check for a comma
Remove extraneous parens. Add comment on logic detecting place_id as parameter.
|
Thanks for your patience, @stephenmcd. Let me know if we need further changes! |
|
No problem at all, thanks for actioning all the feedback! |
Support reverse_geocode by place_id
Add support for reverse geocoding by place_id in addition to existing lat/lng. Doesn't change interface, just checks for a string that begins with something other than a digit or a +/- sign. Treats such strings as place_ids, otherwise behaves like current code.