Question regarding the dodgr_streetnet class

[agila5]

Hi @mpadge, I hope you're doing well! It's been a while since I last created an issue here 😄

I'm opening this issue/discussion because I have a question regarding the definition of dodgr_streetnet objects, such as those created by the weight_streetnet() function. Currently, the weight_streetnet() function sets the class of the output by appending the "dodgr_streetnet" string to the end of the class definition:

https://github.com/UrbanAnalyst/dodgr/blob/13a6b38577cd67898144ab7ca138d42bbf3c6c6e/R/weight-streetnet.R#L307

This results in:

library(dodgr)
net <- weight_streetnet (hampi)
class(net)
#> [1] "data.frame"      "dodgr_streetnet"

While this approach works, I believe it introduces some limitations. For example:

1. It's not possible to use any of the dplyr verbs on "dodgr_streetnet" objects:

library(dplyr)
slice(net, 1)
#> Error in `vec_slice()`:
#> ! `x` must be a vector, not a <data.frame/dodgr_streetnet> object.
#> Backtrace:
#>      ▆
#>   1. ├─dplyr::slice(net, 1)
#>   2. ├─dplyr:::slice.data.frame(net, 1)
#>   3. │ ├─dplyr::dplyr_row_slice(.data, loc, preserve = .preserve)
#>   4. │ └─dplyr:::dplyr_row_slice.data.frame(.data, loc, preserve = .preserve)
#>   5. │   ├─dplyr::dplyr_reconstruct(vec_slice(data, i), data)
#>   6. │   │ └─dplyr:::dplyr_new_data_frame(data)
#>   7. │   │   ├─row.names %||% .row_names_info(x, type = 0L)
#>   8. │   │   └─base::.row_names_info(x, type = 0L)
#>   9. │   └─vctrs::vec_slice(data, i)
#>  10. └─vctrs:::stop_scalar_type(`<fn>`(`<df[,16]>`), "x", `<env>`)
#>  11.   └─vctrs:::stop_vctrs(...)
#>  12.     └─rlang::abort(message, class = c(class, "vctrs_error"), ..., call = call)

This issue could be resolved if class(net) were defined as c("dodgr_streetnet", "data.frame"), which allows dplyr operations:

class(net) <- rev(class(net))
slice(net, 1)
#>   geom_num edge_id   from_id from_lon from_lat     to_id   to_lon   to_lat
#> 1        1       1 339318500 76.47491 15.34167 339318502 76.47612 15.34173
#>          d d_weighted highway   way_id component bicycle     time time_weighted
#> 1 130.0002   144.4447    path 28565950         1    <NA> 39.00007      43.33341

^{Created on 2024-09-18 with reprex v2.0.2}

2. The current class definition also makes it difficult to write S3 methods for "dodgr_streetnet" when there is potential for conflict with "data.frame" methods. See here for an example.

Would you be open to considering a change to c("dodgr_streetnet", "data.frame") for future versions (perhaps dodgr v1.0)? I feel this approach better reflects the idea that "dodgr_streetnet" is a subclass of "data.frame", as described here.

Looking forward to your thoughts on this!

[mpadge]

Thanks @agila5, and great to hear from you once again! That was very easy to fix, and doesn't affect anything here. That said, it it odd that I've never noticed anything not working with dplyr, because as far as I knew, that always followed NextMethod anyway, so should always have dispatched on data.frame alone. I only ever did stuff like this, which seemed fine (where this first code is previous version with class of c("data.frame", "dodgr_streetnet")):

library (dodgr)
packageVersion ("dodgr")
#> [1] '0.4.1.17'
library (dplyr)
net <- weight_streetnet (hampi)
net |> group_by (way_id) |> summarise (d = sum (d))
#> # A tibble: 236 × 2
#>    way_id        d
#>    <chr>     <dbl>
#>  1 123463595 2620.
#>  2 123463598 9390.
#>  3 123463601 1359.
#>  4 126094049 2647.
#>  5 129323700  175.
#>  6 129323701  463.
#>  7 129325638  288.
#>  8 129325640  562.
#>  9 154318162  542.
#> 10 159460406  169.
#> # ℹ 226 more rows

But it does indeed error as you suggest:

net |> slice (1L)
#> Error in `vec_slice()`:
#> ! `x` must be a vector, not a <data.frame/dodgr_streetnet> object.

Whatever the reason for that, the changes you suggested now ensure that it all works as expected:

setwd ("/<local>/<path>/<to>/dodgr")
devtools::load_all (".", export_all = TRUE)
#> ℹ Loading dodgr
packageVersion ("dodgr")
#> [1] '0.4.1.26'

net <- weight_streetnet (hampi)
net |> slice (1L)
#>   geom_num edge_id   from_id from_lon from_lat     to_id   to_lon   to_lat
#> 1        1       1 339318500 76.47491 15.34167 339318502 76.47612 15.34173
#>          d d_weighted highway   way_id component bicycle     time time_weighted
#> 1 130.0002   144.4447    path 28565950         1    <NA> 39.00007      43.33341

^{Created on 2024-09-19 with reprex v2.1.1}

So clearly an improvement, thanks to you!

[agila5]

Thanks! I think this is really useful since that also implies we can define st_as_sf.dodgr_streetnet, as_sfnetwork.dodgr_streetnet & friends.