dodgr_isoverts with sf input and dlim argument

[diegoteca]

Hi Mark,

I have a specific problem/doubt with _dodgrisoverts function, and I don't know if it is documentation error, or it is one desired effect by design. However, this doubt can extrapolate for other functions too.

Is it possible run the function _dodgrisoverts with a data_frame or sf object as input? If one read the documentation of this function it says “graph = data.frame or equivalent object representing the network graph (see Notes)” but when I run the code with hampi data (see reprex below) I receive this message:

Error in dodgr_isoverts(graph, from = from, dlim) : “isoverts can only be calculated from SC-class networks generated by osmdata_sc.”

For my particular case, I’m interested in use _dodgrisoverts with “dlim” argument (and not primarily the “tlim” argument). I highlight this difference because _dodgrisochrones at this moment don’t work with sf_format so I could expect that _dodgrisoverts with “tlim” doesn't work either. Instead, _dodgrisodists works with sf_format so I could expect than _dodgr_isovert_s whith “dlim” work with sf_format.

More general, I created this table for summary what documentation says about diferents functions and what is the behavior in the reprex with hampi's data.

Function	Documentation says	Real behavior
dodgr_dists	data.frame or equivalent object representing the network graph (see Notes)	Work with SF and SC format
dodgr_times	A dodgr network returned from the weight_streetnet function using a network obtained with the osmdata  osmdata_sc function, possibly contracted with dodgr_contract_graph.	Work with SF and SC format
dodgr_isodists	data.frame or equivalent object representing the network graph (see Notes)	Work with SF and SC format
dodgr_ isochrones	data.frame or equivalent object representing the network graph (see Notes)	Work only with SC format
dodgr_isoverts	data.frame or equivalent object representing the network graph (see Notes)	Work only with SC format

library(dodgr)
library(osmdata)
#> Data (c) OpenStreetMap contributors, ODbL 1.0. https://www.openstreetmap.org/copyright

# Build Graphs ----

graph_sf = weight_streetnet(hampi)

graph_sc = opq("hampi india") |>
    add_osm_feature(key = "highway") |>
    osmdata_sc() |>
          weight_streetnet()
#> Loading required namespace: dplyr

# Sample From ----

from_sf = sample(graph_sf$from_id, size = 50)
from_sc = sample(graph_sc$.vx0, size = 50)

# Sample To ---

to_sf <- sample(graph_sf$to_id, size = 50)
to_sc <- sample(graph_sc$.vx1, size = 50)

# Set distance and time ----

dlim = c(2500, 5000) # Distances in meters
tlim = c(5, 10) * 60 # Times in seconds

# Test distances ----

# Work!
distance_sf <- dodgr_dists(graph_sf, from = from_sf, to = to_sf)

# Work!
distance_sc <- dodgr_dists(graph_sc, from = from_sc, to = to_sc)

# Test times ----

# Work!
times_sf <- dodgr_times(graph_sf, from = from_sf, to = to_sf)

# Work!
times_sc <- dodgr_times(graph_sc, from = from_sc, to = to_sc)

# Test isodists ----

# Work!
isodist_sf = dodgr_isodists(graph_sf, from = from_sf, dlim)

# Work!
isodist_sc = dodgr_isodists(graph_sc, from = from_sc, dlim)

# Test isochrones ----

# Don't work!
isochrones_sf = dodgr_isochrones(graph_sf, from = from_sf, tlim)
#> Error in dodgr_isochrones(graph_sf, from = from_sf, tlim): isochrones can only be calculated from SC-class networks generated by osmdata_sc.

# Work!
isochrones_sc = dodgr_isochrones(graph_sc, from = from_sc, tlim)

# Test Isoverts ----

# Don't work!
isoverts_sf = dodgr_isoverts(graph_sf, from = from_sf, dlim)
#> Error in dodgr_isoverts(graph_sf, from = from_sf, dlim): isoverts can only be calculated from SC-class networks generated by osmdata_sc.

# Work!
isoverts_sc = dodgr_isoverts(graph_sc, from = from_sc, dlim)

^{Created on 2023-12-12 with reprex v2.0.2}

[mpadge]

H @diegoteca, thank you for already thinking deeply about this issue. And yes, you are right that this is "by design". The main problem is that storing as sf requires stripping out a lot of important information, so any routing queries with sf-format data are only approximations of more accurate SC versions. Given that, I decided that isovert/isochrone algorithms would only be implemented for SC, because the sf values would be too unreliable.

It would be great to have that table somewhere in the documentation. If you see a good place for it, please feel most welcome to submit a pull request.

[diegoteca]

Hi Mark!

Thanks for you quick reply!

I will think where we could put the table. Maybe into the "Important note" section of Readme? Perhaps with one paragraph about this is enough. Or maybe make a new vignette for this specific problem about the input (sf or sc) of some functions?

On the other hand, if the reason of the sf/sc formats inputs in each function is "by design", I think that is convenient previously edit the documentation of each function. More concrete, in the Roxygen comments about "@param graph" because is that not match very good with the real behaviour. I can do that in the next days.

In this case, after this changes and, regardless of where could go the table, we could updates the table's content. So, the table's content would look some like this:

Function	Documentation says	Real behavior
dodgr_dists	data.frame or equivalent object representing the network graph (see Notes)	Work with SF and SC format
dodgr_times	data.frame or equivalent object representing the network graph (see Notes)	Work with SF and SC format
dodgr_isodists	data.frame or equivalent object representing the network graph (see Notes)	Work with SF and SC format
dodgr_ isochrones	A dodgr network returned from the weight_streetnet function using a network obtained with the osmdata  osmdata_sc function, possibly contracted with dodgr_contract_graph.	Work only with SC format
dodgr_isoverts	A dodgr network returned from the weight_streetnet function using a network obtained with the osmdata  osmdata_sc function, possibly contracted with dodgr_contract_graph.	Work only with SC format

[mpadge]

That sounds great Diego, thanks. Updating the param descriptions could be a little tricky, as a lot of those are inherited (that is, filled via ˋ@inheritParamsˋ hooks). Feel free to have a go updating those if you like, otherwise i'll get on to it as soon as i can.

[diegoteca]

Hi Mark!

I don't have much experience with Roxygen2 ecosystem (specific with '@inheritParams' hooks) and perhaps I will generate more crash than benefits with my pull request.

In this case, I consider that if you could make this change (of course, when you can) is will be better for all.

Thanks for this great package!