-
Notifications
You must be signed in to change notification settings - Fork 3
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
geocodePL_get returns sf data.frame
#43
Conversation
output = jsonlite::fromJSON(prepared_URL)[["results"]]
if (!is.null(output)) {
|
Yes, it can be. For example when address was not found > address = "Poznań, Aleja Niepodległości"
> base_URL = "https://services.gugik.gov.pl/uug/?request=GetAddress&address="
> prepared_URL = paste0(base_URL, address)
> prepared_URL = gsub(" ", "%20", prepared_URL)
> output = jsonlite::fromJSON(prepared_URL)[["results"]]
> output
NULL |
Good catch! |
I noticed that it is very sensitive to the address specification. See for instance the following examples: > address = "Poznań, Ząbkowicka 12H"
> base_URL = "https://services.gugik.gov.pl/uug/?request=GetAddress&address="
> prepared_URL = paste0(base_URL, address)
> prepared_URL = gsub(" ", "%20", prepared_URL)
> output = jsonlite::fromJSON(prepared_URL)[["results"]]
> str(output,1)
List of 1
$ 1:List of 16
> address = "Poznań, Zabkowicka 12H"
> base_URL = "https://services.gugik.gov.pl/uug/?request=GetAddress&address="
> prepared_URL = paste0(base_URL, address)
> prepared_URL = gsub(" ", "%20", prepared_URL)
> output = jsonlite::fromJSON(prepared_URL)[["results"]]
> output
> str(output,1)
List of 1
$ 1:List of 16
> address = "Poznań, Ząbkowicka 12 H"
> base_URL = "https://services.gugik.gov.pl/uug/?request=GetAddress&address="
> prepared_URL = paste0(base_URL, address)
> prepared_URL = gsub(" ", "%20", prepared_URL)
> output = jsonlite::fromJSON(prepared_URL)[["results"]]
> output
NULL
> address = "Poznań Ząbkowicka 12H"
> base_URL = "https://services.gugik.gov.pl/uug/?request=GetAddress&address="
> prepared_URL = paste0(base_URL, address)
> prepared_URL = gsub(" ", "%20", prepared_URL)
> output = jsonlite::fromJSON(prepared_URL)[["results"]]
> output
NULL |
Suppose output = jsonlite::fromJSON(prepared_URL)[["results"]] returns NULL. In that case, |
Yes, see below > geocodePL_get(address = "Poznań Ząbkowicka 12H")
Error in geocodePL_get(address = "Poznań Ząbkowicka 12H") :
all inputs are empty |
So that's not right. This error should appear after the user doesn't enter the input. If |
Ok, so I should include the if (!nchar(rail_crossing) == 11) {
stop("rail crossing ID must be 11 characters long")
} or maybe rearrange the whole function so the |
Maybe just before if (is.null(output)) {
return(NULL)
} |
Oh, sorry. I thought after you commit. It is probably better for the user to be informed that the name is possibly wrong than NULL. |
I haven't checked if |
Ok, please check if my proposal fits your requirements. How should the unit test for wrong or no address look like? Should I use |
In testthat there is |
test = geocodePL_get(geoname = "Jeziorak") doesn't work |
The error is because of different number of elements in each list List of 5
$ 1:List of 13
$ 2:List of 12
$ 3:List of 12
$ 4:List of 13
$ 5:List of 12 So using the standard |
if (length(output) == 1) {
output[[1]][sapply(output[[1]], is.null)] = NA
} Maybe the length condition is too restrictive / unfounded? What if the output consists of multiple lists (like in #43 (comment))? |
I would not like to introduce a new dependency. The 13th extra column is "notes", maybe we'll just remove it in each list and then merge the lists? |
Done. Why the checks are not passed? |
1 and 3 I don't know why. It worked yesterday. test_that("check if object was not found", {
expect_output(t6, "object not found")
}) |
Maybe try change to |
Maybe > expect_output(t6, "object not found")
Error: `t6` produced no output May I use EDIT: what is the difference between |
Probably |
I think I can merge this PR now. Please also add your pearson to DESCRIPTION as If you still have time and you would like to, you can address:
|
Thank you! I will try to help with the development of this package. |
Proposal for a changed output of
geocodePL_get
. Now it returns object of asf data.frame
class instead of alist
.