Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

REF/TST: replace GeocodeFarm with Photon as a geocoding default #2007

Merged
merged 2 commits into from Jul 15, 2021

Conversation

martinfleis
Copy link
Member

Fixes #2005

Copy link
Member

@jdmcbr jdmcbr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! I didn't even notice photon as an option when looking at geocoding services to use as a default for testing. I'd made nominatim the default service in #907, but then (somewhat reluctantly) modified that to geocodefarm in #975 upon realizing that our documentation usage violated the ToS for nominatim. From what I read of photon, this seems like a great default option.

@martinfleis martinfleis changed the title TST: replace GeocodeFarm with Photon REF/TST: replace GeocodeFarm with Photon as a geocoding default Jul 14, 2021
@martinfleis
Copy link
Member Author

@jdmcbr you made me realise that changing tests is not enough. Changed the default and docs as well now.

@martinfleis martinfleis requested a review from jdmcbr July 14, 2021 16:16
Copy link
Member

@jdmcbr jdmcbr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@martinfleis sorry, I should have noticed and noted the need to update the documentation myself when I commented. I had instead just looked at photon and their terms of service to confirm it as a good option for us to default to. Anyway, took a more careful look now, and I think you caught everything that needs changing.

@martinfleis martinfleis merged commit 7997837 into geopandas:master Jul 15, 2021
@martinfleis martinfleis deleted the geopy22 branch July 15, 2021 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TST: use a different geocoder than GeocodeFarm in geocoding tests
2 participants