Skip to content
New issue

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

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

Already on GitHub? # to your account

oc_check_query for NAs in placename or latitude/longitude #98

Merged
merged 1 commit into from
Jan 15, 2020

Conversation

dpprdan
Copy link
Member

@dpprdan dpprdan commented Jan 7, 2020

As of recently, OpenCage throws a HTTP 400 ‘bad query’ error when the query is empty, so we cannot be as sneaky as in #89 anymore.
I.e. now we need to make sure that we do not send empty queries (empty placename
or latitude/longitude) to the API, but alert the user beforehand. This is what happens with this PR.

An empty request throws an HTTP 400 error (shown with oc_process() here, but the same happens with oc_forward() and oc_reverse() without this fix):

library(opencage)
opencage:::oc_process("", return = "df_list")
#> Error: HTTP failure: 401
#> missing API key

Instead, oc_check_query() now throws an error before moving to oc_process()/calling the API:

oc_forward("")
#> Error: `placename` must not be NA or an empty string.
oc_reverse(NA_real_, NA_real_)
#> Error: `latitude` must not be NA.

I was a bit reluctant to let the whole call error out ‘just’ because a user provides an empty/NA placename/coordinates, but this is a) what the API effectively demands now and b) essentially the same as providing incorrect coordinates, e.g. outside the (-)90/(-)180 bounds:

oc_reverse(100, 200)
#> Error: Every `latitude` must be between -90 and 90.

OpenCage now throws a HTTP 400 'bad query' error when the query is empty
Copy link
Member

@maelle maelle left a comment

Choose a reason for hiding this comment

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

Thanks a lot! 🚀

tests/testthat/test-oc_check_status.R Show resolved Hide resolved
@dpprdan dpprdan merged commit 6c2112a into ropensci:devel Jan 15, 2020
@dpprdan dpprdan deleted the fix/na_not_allowed branch January 15, 2020 15:32
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants