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

Add dispatch mechanism for geocoding backends #37

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

cosi1
Copy link
Collaborator

@cosi1 cosi1 commented Feb 9, 2019

Folks,

here's a proposed geocoding mechanism that provides a set of predefined backends (currently Nominatim and Google), as well as the ability to use a custom geocoding function. It definitely may need some polishing (no unit tests or a detailed documentation so far) - think of it as of a proof of concept rather than an ultimate solution :-)

Best,
-p-

@zkamvar
Copy link
Member

zkamvar commented Feb 13, 2019

Folks,

here's a proposed geocoding mechanism that provides a set of predefined backends (currently Nominatim and Google), as well as the ability to use a custom geocoding function. It definitely may need some polishing (no unit tests or a detailed documentation so far) - think of it as of a proof of concept rather than an ultimate solution :-)

Best,
-p-

nominatim should not be in here since it's not on CRAN.

Because it seems that there are no geocoding APIs that will work anonymously, then I think we should just pick the smallest package that works and go with that. If users want the geocoding, then they will have to register for a key in any way they can.

@cosi1
Copy link
Collaborator Author

cosi1 commented Feb 13, 2019

@zkamvar Well, it's in Suggested, so it's not a strict requirement, but I get your point.
If we assume that it's up to the user to geocode their locations (and get an API key if need be), we may as well stay with ggmap. The only downside of this approach is that it might be impossible (or at least difficult) to unit-test that function - unless we manually create a mockup response from Google's API and provide a testing switch in add_coordinates().

@zkamvar
Copy link
Member

zkamvar commented Feb 14, 2019

@zkamvar Well, it's in Suggested, so it's not a strict requirement, but I get your point.
If we assume that it's up to the user to geocode their locations (and get an API key if need be), we may as well stay with ggmap.

agreed.

The only downside of this approach is that it might be impossible (or at least difficult) to unit-test that function - unless we manually create a mockup response from Google's API and provide a testing switch in add_coordinates().

I believe this is the premise of the vcf or webmockr packages.

@cosi1
Copy link
Collaborator Author

cosi1 commented Feb 14, 2019

@zkamvar Thanks, I didn't know that vcr had been ported to R!

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.

2 participants