Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test the presence of from and to colnames #272

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Conversation

agila5
Copy link
Collaborator

@agila5 agila5 commented Jul 12, 2024

As described in #271, we currently observe the following cryptic error message when creating a sfnetwork object without the necessary from and to columns:

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

p1 = st_point(c(7, 51))
p2 = st_point(c(7, 52))
l1 = st_sfc(st_linestring(c(p1, p2)))

edges = st_as_sf(data.frame(X1 = "A", X2 = 1, X3 = 2), geometry = l1, crs = 4326)
nodes = st_as_sf(c(st_sfc(p1), st_sfc(p2)), crs = 4326)

(sfn <- sfnetwork(nodes, edges, message = FALSE))
#> Error in (function (edges, n = max(edges), directed = TRUE) : At rinterface_extra.c:82 : The value nan is not representable as an integer. Invalid value

Created on 2024-07-12 with reprex v2.0.2

After the commit included in this PR, we see the following

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

p1 = st_point(c(7, 51))
p2 = st_point(c(7, 52))
l1 = st_sfc(st_linestring(c(p1, p2)))

edges = st_as_sf(data.frame(X1 = "A", X2 = 1, X3 = 2), geometry = l1, crs = 4326)
nodes = st_as_sf(c(st_sfc(p1), st_sfc(p2)), crs = 4326)

(sfn <- sfnetwork(nodes, edges, message = FALSE))
#> Error: Edges are spatially explicit, but 'from' and 'to' columns are missing

edges = st_as_sf(data.frame(X1 = "A", from = 1, to = 2), geometry = l1, crs = 4326)
(sfn <- sfnetwork(nodes, edges, message = FALSE))
#> # A sfnetwork with 2 nodes and 1 edges
#> #
#> # CRS:  EPSG:4326 
#> #
#> # A rooted tree with spatially explicit edges
#> #
#> # A tibble: 2 × 1
#>             x
#>   <POINT [°]>
#> 1      (7 51)
#> 2      (7 52)
#> #
#> # A tibble: 1 × 4
#>    from    to X1            geometry
#>   <int> <int> <chr> <LINESTRING [°]>
#> 1     1     2 A         (7 51, 7 52)

Created on 2024-07-12 with reprex v2.0.2

A couple of comments:

  1. I can include a proper unit test if you think that this change is a good idea;
  2. I'm not 100% sure that we should error in this case, maybe a warning is enough... The reason is that the underlying tidygraph code may work even if the edges table does not contain any column which is named from and to:

https://github.com/thomasp85/tidygraph/blob/382c2b7eec121b7bd842b8acb4209367b4943d74/R/list.R#L72-L75

@agila5 agila5 changed the base branch from main to develop July 12, 2024 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant