ropensci / osmextract

Download and import OpenStreetMap data from Geofabrik and other providers
https://docs.ropensci.org/osmextract
GNU General Public License v3.0
167 stars 12 forks source link

[BUG/DOCUMENTATION DISCREPENCY] oe_get_network() documentation discrepancy #254

Closed hulsiejames closed 2 years ago

hulsiejames commented 2 years ago

Describe the bug documentation of the oe_get_network() function (through help(oe_get_network) ) implies support for multiple mode selection through combining values but this does not seem the case

To Reproduce Describe the steps used to reproduce the bug. Please note that including a reproducible example (consider using a "reprex") can be extremely helpful for diagnosing the problem and solving the bug.

Viewing documentation through help(oe_get_network) I see

Usage 
oe_get_network(place, mode = c("cycling", "driving", "walking"), ...) 

So trying the following should return the combined networks of cycling, driving and walking

> new_test_region = osmextract::oe_get_network(
+   place = region,
+   mode = c('walking', 'driving', 'cycling'),
+   provider = 'bbbike',
+   force_download = T,
+   force_vectortranslate = T
+ )

Yet in practice returns an argument length error

Error in match.arg(mode) : 'arg' must be of length 1

I'm not sure if this is the expected behaviour and the documentation example is outdated/incorrect or if the documentation is correct and there is an issue with the eo_get_network() function.

I am assuming the former as the function works perfectly fine with a single input argument rather than a combined character vector

Expected behaviour From the help documentation, I would expect a custom network to be returned consisting of the walking, driving and cycling networks being returned as a single network

Additional context No additional context but find sessioninfo below.

Session Info ```r > sessionInfo() R version 4.1.3 (2022-03-10) Platform: x86_64-pc-linux-gnu (64-bit) Running under: Ubuntu 20.04.4 LTS Matrix products: default BLAS: /usr/lib/x86_64-linux-gnu/blas/libblas.so.3.9.0 LAPACK: /usr/lib/x86_64-linux-gnu/lapack/liblapack.so.3.9.0 locale: [1] LC_CTYPE=en_GB.UTF-8 LC_NUMERIC=C LC_TIME=en_GB.UTF-8 LC_COLLATE=en_GB.UTF-8 LC_MONETARY=en_GB.UTF-8 [6] LC_MESSAGES=en_GB.UTF-8 LC_PAPER=en_GB.UTF-8 LC_NAME=C LC_ADDRESS=C LC_TELEPHONE=C [11] LC_MEASUREMENT=en_GB.UTF-8 LC_IDENTIFICATION=C attached base packages: [1] stats graphics grDevices utils datasets methods base other attached packages: [1] arsenal_3.6.3 tmap_3.3-3 osmextract_0.4.0 sf_1.0-7 forcats_0.5.1 stringr_1.4.0 dplyr_1.0.8 purrr_0.3.4 [9] readr_2.1.2 tidyr_1.2.0 tibble_3.1.6 ggplot2_3.3.5 tidyverse_1.3.1 loaded via a namespace (and not attached): [1] httr_1.4.2 jsonlite_1.8.0 viridisLite_0.4.0 modelr_0.1.8 assertthat_0.2.1 sp_1.4-7 cellranger_1.1.0 [8] pillar_1.7.0 backports_1.4.1 lattice_0.20-45 glue_1.6.2 digest_0.6.29 RColorBrewer_1.1-3 rvest_1.0.2 [15] colorspace_2.0-3 htmltools_0.5.2 XML_3.99-0.9 pkgconfig_2.0.3 broom_0.8.0 raster_3.5-15 haven_2.5.0 [22] stars_0.5-5 scales_1.2.0 terra_1.5-21 tzdb_0.3.0 proxy_0.4-26 generics_0.1.2 ellipsis_0.3.2 [29] withr_2.5.0 leafsync_0.1.0 cli_3.2.0 magrittr_2.0.3 crayon_1.5.1 readxl_1.4.0 fs_1.5.2 [36] fansi_1.0.3 xml2_1.3.3 lwgeom_0.2-8 class_7.3-20 tools_4.1.3 hms_1.1.1 lifecycle_1.0.1 [43] munsell_0.5.0 reprex_2.0.1 compiler_4.1.3 e1071_1.7-9 rlang_1.0.2 classInt_0.4-3 units_0.8-0 [50] grid_4.1.3 tmaptools_3.1-1 rstudioapi_0.13 dichromat_2.0-0.1 htmlwidgets_1.5.4 crosstalk_1.2.0 leafem_0.2.0 [57] base64enc_0.1-3 gtable_0.3.0 codetools_0.2-18 curl_4.3.2 abind_1.4-5 DBI_1.1.2 R6_2.5.1 [64] lubridate_1.8.0 fastmap_1.1.0 utf8_1.2.2 KernSmooth_2.23-20 stringi_1.7.6 parallel_4.1.3 Rcpp_1.0.8.3 [71] vctrs_0.4.1 png_0.1-7 leaflet_2.1.1 dbplyr_2.1.1 tidyselect_1.1.2 ```
agila5 commented 2 years ago

Hi @hulsiejames and thanks for your message.

documentation of the oe_get_network() function (through help(oe_get_network) ) implies support for multiple mode selection through combining values but this does not seem the case

I'm sorry for the misunderstanding, but oe_get_network() only supports a unique mode of transport at a time. I could update the description of the mode argument as follows:

mode: A character string of length 1 denoting the desired mode of transport. Can be abbreviated. Currently cycling (the default), driving and walking are supported.

Do you think that's more clear?

From the help documentation, I would expect a custom network to be returned consisting of the walking, driving and cycling networks being returned as a single network

I think that it might be quite difficult to define general criteria to return OSM ways that satisfy multiple conditions (i.e. walking or cycling), but, if you want, I can show you an example using a more manual approach (i.e. oe_get() + an ad-hoc query).

hulsiejames commented 2 years ago

Hi @agila5 thank you for the very fast response!

No worries at all - I was just concerned that I might not be understanding the documentation as intended (this often happens and seemingly simple concepts can confuse me!)

I would agree that the updated mode description is more clear and would definitely remove the ambiguity I felt!

I have had some experience creating custom networks using oe_get(layer='lines') and then filtering this down by filtering out any tags not relevant to the network I wish to create - but if there is an approach which can do this by querying the type of network features I would like rather than requesting many features and filtering out the not-required ones it would be greatly appreciated.

Many thanks.

Robinlovelace commented 2 years ago

Good to see this and definitely links to the OpenInfra project. From the perspective of this issue, I think this message is pretty clear, it can only take one mode:

Error in match.arg(mode) : 'arg' must be of length 1

(OK maybe the message could be a bit clearer but the docs state that it's only one.)

Does that sound reasonable @hulsiejames ? I suggest closing this issue if so, look forward to catching up on this + more.

Robinlovelace commented 2 years ago

This may be best placed in the Discussion section: https://github.com/ropensci/osmextract/discussions

hulsiejames commented 2 years ago

Agreed @Robinlovelace! Will close as not sure how to migrate to discussions.

agila5 commented 2 years ago

Hi @hulsiejames. I copy here an example that shows how to select OSM ways that satisfy some criteria for walking/cycling:

osmextract::oe_get(
  place = "Isle of Wight", 
  layer = "lines", 
  extra_tags = c("access", "bicycle", "service", "foot"), 
  query = "
  SELECT * 
  FROM lines 
  WHERE 
  (highway is NOT NULL)
  AND
  (highway NOT IN ('abandonded', 'bus_guideway', 'byway', 'construction',
  'corridor', 'elevator', 'fixme', 'escalator', 'gallop', 'historic', 'no', 
  'planned', 'platform', 'proposed', 'raceway'))
  AND
  (access IS NULL OR access NOT IN ('private', 'no'))
  AND 
  (bicycle IS NULL or 
  foot IS NULL or 
  bicycle NOT IN ('private', 'no', 'use_sidepath', 'restricted') or 
  foot NOT IN ('private', 'no', 'use_sidepath', 'restricted'))
  "
)
#> The input place was matched with: Isle of Wight
#> The chosen file was already detected in the download directory. Skip downloading.
#> The corresponding gpkg file was already detected. Skip vectortranslate operations.
#> Reading query `
#>   SELECT * 
#>   FROM lines 
#>   WHERE 
#>   (highway is NOT NULL)
#>   AND
#>   (highway NOT IN ('abandonded', 'bus_guideway', 'byway', 'construction',
#>   'corridor', 'elevator', 'fixme', 'escalator', 'gallop', 'historic', 'no', 
#>   'planned', 'platform', 'proposed', 'raceway'))
#>   AND
#>   (access IS NULL OR access NOT IN ('private', 'no'))
#>   AND 
#>   (bicycle IS NULL or 
#>   foot IS NULL or 
#>   bicycle NOT IN ('private', 'no', 'use_sidepath', 'restricted') or 
#>   foot NOT IN ('private', 'no', 'use_sidepath', 'restricted'))
#>   ' from data source `C:\Users\gilardi\Documents\osm-data\geofabrik_isle-of-wight-latest.gpkg' 
#>   using driver `GPKG'
#> Simple feature collection with 19108 features and 13 fields
#> Geometry type: LINESTRING
#> Dimension:     XY
#> Bounding box:  xmin: -1.582318 ymin: 50.57523 xmax: -1.069921 ymax: 50.76732
#> Geodetic CRS:  WGS 84

Created on 2022-05-04 by the reprex package (v2.0.1)

Clearly, you should adjust the criteria according to your needs (see also here: https://github.com/ropensci/osmextract/blob/master/R/get-network.R), but I think this is a good starting point!