UrbanAnalyst / dodgr

Distances on Directed Graphs in R
https://urbananalyst.github.io/dodgr/
128 stars 16 forks source link

Unclear error message when sf object has non-LINESTRING geometry #246

Closed agila5 closed 2 months ago

agila5 commented 2 months ago

Hallo again @mpadge! I'm creating this issue to highlight the following slightly problematic behaviour:

library(dodgr)
library(sf)
#> Linking to GEOS 3.11.2, GDAL 3.7.2, PROJ 9.3.0; sf_use_s2() is TRUE

# toy data 
toy <- st_as_sf(
  data.frame(highway = "primary"), 
  geometry = st_sfc(
    st_linestring(rbind(c(0, 0), c(1, 1))), 
    st_multilinestring(
      list(
        st_linestring(rbind(c(1, 1), c(2, 2)))
      )
    )
  )
)

# The following fails with unclear error message
weight_streetnet(toy)
#> x appears to have no ID column; sequential edge numbers will be used.
#> Error in eval(expr, envir, enclos): Not compatible with requested type: [type=list; target=double].
# Now everything works fine
weight_streetnet(toy[1, ])
#> x appears to have no ID column; sequential edge numbers will be used.
#>   geom_num edge_id from_id from_lon from_lat to_id to_lon to_lat        d
#> 1        1       1       0        0        0     1      1      1 156899.6
#> 2        1       2       1        1        1     0      0      0 156899.6
#>   d_weighted highway way_id component    time time_weighted
#> 1   224142.2 primary      1         1 37655.9      53794.14
#> 2   224142.2 primary      1         1 37655.9      53794.14

Created on 2024-09-23 with reprex v2.0.2

I'm not sure if it's possible to adjust the behaviour behind weight_streetnet() to subset only LINESTRING geometries but, IMO, this type of interaction deserves a slightly more informative error message. Happy to create a PR if you agree.

mpadge commented 2 months ago

Thanks @agila5, a PR would indeed be great! Note in preparation for that that sf is not directly used at all, except in the dodgr_to_sf() conversion function. I'd prefer to avoid depending on it for the central functions if at all possible, so trying to do this without direct sf usage would be appreciated. Since you're only interested in distinguishing sf types, that should be fairly easy, and those can be obtained directly from the attributes.


Update: You could easily put code here: https://github.com/UrbanAnalyst/dodgr/blob/ac1eec58a947eab2c89fb04a600b8417ec6f3460/R/weight-streetnet.R#L238-L239 right before the call to rcpp_sf_as_network. Something like this should do:

geom_types <- lapply (x$geometry,  class)

And then filter out anything that's not a LINESTRING.