GIScience / openrouteservice-r

:globe_with_meridians: query openrouteservice API from R
https://giscience.github.io/openrouteservice-r/
Apache License 2.0
98 stars 23 forks source link

Adapt interface in order to improve package interoperability #42

Closed Robinlovelace closed 5 years ago

Robinlovelace commented 5 years ago

First to say: many thanks for a great package!

Wondering if you have plans to submit it to CRAN. Context: thinking of writing a wrapper, e.g. route_ors(), in stplanr. We've written some code here that uses it, rather inefficiently at the moment!: https://github.com/pedalea/pctSantiago

aoles commented 5 years ago

Dear Robin,

many thanks for reaching out. Awesome job with stplanr!

Sure, the ultimate goal is to publish the package on CRAN. Before that there are still some issues which I believe need to be addressed. In particular, as I consider the package to be in development stage there might be some slight changes to the package API like adjustments to function names or function arguments such as https://github.com/GIScience/openrouteservice-r/issues/39 and https://github.com/GIScience/openrouteservice-r/issues/33.

Which leads me to the question: what is the primary need for route_ors wrapper? If the problem is the structuring of input/output formats I'm open to discussion of possible improvements on our side for better interoperability of packages.

Cheers, Andrzej

Robinlovelace commented 5 years ago

Hey @aoles great to hear. Yes it's more a question of output. I suspect many R users will find it useful for the function to return an sf object. In terms of inputs... That's a different matter - there' a need for consistency and @mem48 have discussed ways of doing this, with matrices of origin-destination pairs being a useful input format I think. If it returns sf linestrings, stplanr could just import it and use it as another option in route(FUN = ...) calls would be the plan. See https://github.com/ropensci/stplanr/issues/283 for more on my plans going forward.

aoles commented 5 years ago

Thanks a lot for your feedback!

The input to ors_directions is in a way predefined by openrouteservice API itself as the original idea of the package was to provide easy access to our services from R. But we could of course extend the R interface by adding from and to arguments as an alternative way of specifying origin-destination pairs. As a side note, we could maybe do the same for ors_matrix: provide an alternative way of specifying the pairing by from and to arguments rather than locations and destinations.

Regarding the output format I will explore ways of improving support for sf objects. Of course, the easiest option would be to extend the list of possible output formats of ors_directions to return a linestring. However, considering that openrouteservice response is much more complex than just the route geometry, I'm not entirely sure that this is the cleanest solution. I need to give it a thought, so stay tuned.

looking forward to the exciting collaboration between our packages ;)

Cheers, Andrzej

Robinlovelace commented 5 years ago

Great. I can have a first bash at implementing output = ors_sf (which should contain the attribute and geometry info).

aoles commented 5 years ago

Dear Robin,

many thanks for taking the time to look into this and for submitting your pull request. After careful consideration I have decided to adapt some of the package interface in order to natively support sf output. Feel free to give it a try on the output_sf branch.

Cheers, Andrzej

Robinlovelace commented 5 years ago

Great work. Not tried but, after looking at the code, it seems like a sensible solution. One question: is it possible to send a list of requests to be routed? For example, by sending a matrix of origin-destination pairs (I know you can send a list of waypoints but what about many trips that start and end in different places?) like this, to calculate the routes for 2 lines:

stplanr::line2df(stplanr::flowlines_sf[2:3, ])
#> # A tibble: 2 x 5
#>      L1    fx    fy    tx    ty
#>   <dbl> <dbl> <dbl> <dbl> <dbl>
#> 1     1 -1.52  53.8 -1.54  53.8
#> 2     2 -1.52  53.8 -1.55  53.8

Created on 2019-04-11 by the reprex package (v0.2.1)

In stplanr you can currently do this as follows:

library(stplanr)
r = line2route(flowlines_sf[2:3, ], route_fun = route_cyclestreet)
plot(r)
#> Warning: plotting the first 10 out of 17 attributes; use max.plot = 17 to
#> plot all

Created on 2019-04-11 by the reprex package (v0.2.1)

aoles commented 5 years ago

Dear Robin,

thanks for your feedback! The openrouteservice routing API which ors_directions() interfaces is designed to take just a single route at a time. Considering the fact that the package aims to act as a lightweight client to the API I believe that covering more complicated input scenarios is not our highest priority at the moment, I'm afraid. Rather, we would like to focus on addressing any remaining issues before releasing the package on CRAN.

In particular, querying for a list of routes is relatively simple to implement by the end user. In contrary, a vectorized package function might easily become complicated and quite obscure considering all the possible route configuration options. Because of that I don't see an urgent need for addressing this issue by designing a generic solution in the package.

For now I would suggest that you consider providing a route_ors() wrapper in stplanr (probably similar to route_graphhopper()) and handle multi-route request on your side.

Cheers, Andrzej

Robinlovelace commented 5 years ago

That sounds reasonable - batch routing is a specific task that can make life more complicated so happy to approach this on the stplanr end.

Many thanks for the reply, and excellent work on support for sf outputs. I think this issue can be closed when the https://github.com/GIScience/openrouteservice-r/tree/output_sf is merged into master. Any further feedback you'd like from me: just say.

Robinlovelace commented 5 years ago

Great work @aoles many thanks.