search

dieghernan / nominatimlite

Lite interface for getting data from OSM geocoder service.
https://dieghernan.github.io/nominatimlite/
Other
19 stars 2 forks source link

interpretation of polygon = TRUE in geo_lite_sf() #8

Closed jlacko closed 3 years ago

jlacko commented 3 years ago

The "polygon" parameter of geo_lite_sf() is IMHO a killer feature. There is a slight catch though: while all OSM objects can be relied on having a point coordinate defined not all of them come as polygons / think of rivers and roads (lines) or amenities (mostly points only).

This is mentioned in passing in documentation ("or potentially other shapes as polygons or lines") but from this it may not be immediately obvious to users that the polygon is not guaranteed.

When you consider this code an user might - going by the name of the parameter only - expect geometry of type polygon. When in fact it is a linestring.

> geo_lite_sf("Gran Via, Madrid", polygon = T)
Simple feature collection with 1 feature and 2 fields
Geometry type: LINESTRING
Dimension:     XY
Bounding box:  xmin: -3.706446 ymin: 40.42041 xmax: -3.706174 ymax: 40.42059
Geodetic CRS:  WGS 84
             query
1 Gran Via, Madrid
                                                                                                                         address
1 Gran Vía, Universidad, Centro, Madrid, Área metropolitana de Madrid y Corredor del Henares, Comunidad de Madrid, 28001, España
                        geometry
1 LINESTRING (-3.706446 40.42...

It is certainly impractical to force the results to a certain geometry type.

But what may be considered is renaming the parameter - to e.g. points_only, defaulting to FALSE - to make the possibility of other geometry types feel more real.

I would be happy to prepare a pull request with the slight amendment of functionality described, but before I proceed I would appreciate your advice on its practicability / desirability.

dieghernan commented 3 years ago

Thanks @jlacko , seems to be completely reasonable. The motivation of using polygon param was that indeed the Nominatim docs uses that convention:

https://github.com/dieghernan/nominatimlite/blob/9697c792cda5f592f3f948d2f6897f0affd13267/R/geo_lite_sf.R#L121-L123

But I completely agree with you. We may change it in the package.

jlacko commented 3 years ago

Oki, I will do that. To be honest this feature is what I like the most on the entire package - as a Linux user I am not troubled too much by the curl dependency, but getting a polygon / line object in a single function call seems a major win (compared to {osmdata}, which is an excellent package giving its users full control and what not, but unpicking all the results is something of a chore).

dieghernan commented 3 years ago

Simple yet effective 😉