Swap OSM tags when retrieving city boundary

[cforgaci]

What type of PR is this? (check all applicable)

• Refactor
• Feature
• Bug Fix
• Optimization
• Documentation Update

Description

With this PR, get_osm_city_boundary first tries to get the data first with the tag boundary:administrative and then place:city. Most city boundaries are recorded (correctly) with the former, so it makes sense to begin with that one. Previously it was the other way around and would return wrong boundaries, like in the case of Bratislava.

Related Issues

• Related Issue Correct OSM city boundary retreival #83
• Closes Correct OSM city boundary retreival #83

Added/updated tests?

We encourage you to keep the code coverage percentage at 75% and above.

• Yes
• No, and this is why: please replace this line with details on why tests
  have not been included
• I need help with writing tests

Added entry in changelog?

For user-facing changes, add a line describing the changes in NEWS.md

• Yes

[fnattino]

Thanks @cforgaci. Just a curiosity: are there any cities for which boundary:administrative does not work and place:city does? If this is not the case we could simply drop the search for the place:city tag ..

[fnattino]

It turns out the size of the boundary/administrative query is quite large (see #177) - do we still want to invert the order?

[cforgaci]

It turns out the size of the boundary/administrative query is quite large (see #177) - do we still want to invert the order?

To be honest, I do not remember why we used place:city. I cannot find any case that does not find a boundary with boundary:administrative. Even if the query is large with that, I would still keep that and drop the place:city alternative. If you agree, @fnattino, I will update the code accordingly.

[fnattino]

Sounds good to only keep boundary:administrative - but maybe let's make the city boundary retrieval optional within get_osmdata())?

My point is that we could have get_osmdata() retrieving by default the city boundary, but within the top-level function delineate() we should avoid retrieving data that is not actually used (and not returned to the user either). So, within delineate(), we could do something like:

osmdata <- get_osmdata(..., city_boundary = FALSE, ...)

[cforgaci]

@fnattino, I updated get_osm_city_boundary() to only use boundary:administrative and I disabled city_boundary retrieval in delineate() as you suggested. Will this work?

[fnattino]

Looks good @cforgaci, just one suggestion, then good to merge on my side!