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

Implement to_spatial_segmentation #210

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Implement to_spatial_segmentation #210

wants to merge 6 commits into from

Conversation

agila5
Copy link
Collaborator

@agila5 agila5 commented Jun 8, 2022

Showcase of the new spatial morpher:

# packages
library(sf)
library(tidygraph)
library(spatstat)
library(sfnetworks)

# simulated data
my_sfc <- st_sfc(
  st_linestring(rbind(c(-1, 0), c(1, 0), c(1, 1), c(1, 2), c(3, 2))), 
  st_linestring(rbind(c(3, 2), c(3, 3))), 
  st_linestring(rbind(c(3, 3), c(3, 4), c(4, 4))), 
  st_linestring(rbind(c(5, 0), c(5, 2), c(5, 5))), 
  st_linestring(rbind(c(0.5, 0.5), c(1.5, 0.5))) 
)
my_sfn <- as_sfnetwork(my_sfc)
par(mfrow = c(1, 2))
plot(my_sfn, graticule = TRUE, axes = TRUE, xlim = c(-2, 6), lwd = 2, cex = 2)
plot(as.linnet(my_sfn), main = "", vertices = TRUE, axes = TRUE, lwd = 2)
#> Warning: Network is not connected

# Run the new morpher and plot
my_sfn1 <- convert(my_sfn, to_spatial_dump_segments, .clean = TRUE)
plot(my_sfn1, graticule = TRUE, axes = TRUE, xlim = c(-2, 6), lwd = 2, cex = 2)
plot(as.linnet(my_sfn1), main = "", vertices = TRUE, axes = TRUE, lwd = 2)
#> Warning: Network is not connected

# Add two fields, CRS and precision
my_sf <- st_sf(
  data.frame(x = c("A", "B", "C", "D", "E"), y = runif(5)), 
  geometry = my_sfc, 
  agr = c(x = "constant"), 
  crs = 3003, 
  precision = 1
)
my_sfn <- as_sfnetwork(my_sf)
my_sfn1 <- convert(my_sfn, to_spatial_dump_segments, .clean = TRUE)
#> Warning: to_spatial_subdivision assumes attributes are constant over geometries

identical(st_crs(my_sfn, "edges"), st_crs(my_sfn1, "edges"))
#> [1] TRUE
identical(st_agr(my_sfn, "edges"), st_agr(my_sfn1, "edges"))
#> [1] TRUE

Created on 2022-06-08 by the reprex package (v2.0.1)

I know that the approach is not perfect (since we could also infer the new nodes without running as_sfnetwork() again), but I think it's good enough for the moment. If you don't have any particular comments, I will add examples and tests as soon as possible.

cc @mkvasnicka

@luukvdmeer
Copy link
Owner

One question: since the network is recreated with as_sfnetwork this would also connect edges that share an interior point right, like to_spatial_subdivision does? I am not sure if that is expected or not

I see the name comes from PostGIS, but I think to_spatial_segmentation would be clearer, considering our other morpher names

Base automatically changed from develop to main August 18, 2022 21:05
@agila5 agila5 changed the base branch from main to develop September 4, 2022 09:32
Merge branch 'develop' into dumpSegments

# Conflicts:
#	NAMESPACE
#	man/spatial_morphers.Rd
@agila5
Copy link
Collaborator Author

agila5 commented Sep 8, 2022

I see the name comes from PostGIS, but I think to_spatial_segmentation would be clearer, considering our other morpher names

Fixed

One question: since the network is recreated with as_sfnetwork this would also connect edges that share an interior point right, like to_spatial_subdivision does? I am not sure if that is expected or not

Do you mean something like the following example?

# packages
library(sf)
library(tidygraph)
library(sfnetworks)


# data
my_sfc <- st_sfc(
  st_linestring(rbind(c(-1, 0), c(0, 0), c(1, 0))), 
  st_linestring(rbind(c(0, -1), c(0, 0), c(0, 1))) 
)
my_sfn <- as_sfnetwork(my_sfc)
my_sfn1 <- convert(my_sfn, to_spatial_segmentation, .clean = TRUE)

# plot
par(mfrow = c(1, 2), mar = rep(0, 4))
plot(my_sfn)
plot(my_sfn1)

Created on 2022-09-08 by the reprex package (v2.0.1)

We can notice that the morpher creates a new "link" between the existing edges (i.e. the new dot in the middle of the map).

I'm not 100% sure that this is expected but:

  1. this is something that will never happen as long as you run the to_spatial_subdivision morpher before this other morpher (and this is particularly relevant for OSM data);
  2. I think we can merge this PR, document this behaviour, and then adjust it if someone explicitly complains (and, meanwhile, create a new issue so we don't forget about it).

@agila5 agila5 changed the title Implement to_spatial_dump_segments Implement to_spatial_segmentation Oct 1, 2022
Base automatically changed from develop to main March 22, 2023 16:08
@codecov-commenter
Copy link

Codecov Report

Patch coverage: 25.00% and project coverage change: -0.63 ⚠️

Comparison is base (d3374b5) 67.92% compared to head (befbf31) 67.29%.

❗ Current head befbf31 differs from pull request most recent head 5fb61a6. Consider uploading reports for the commit 5fb61a6 to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #210      +/-   ##
==========================================
- Coverage   67.92%   67.29%   -0.63%     
==========================================
  Files          21       21              
  Lines        1593     1581      -12     
==========================================
- Hits         1082     1064      -18     
- Misses        511      517       +6     
Impacted Files Coverage Δ
R/morphers.R 70.04% <0.00%> (-4.70%) ⬇️
R/spatstat.R 50.00% <ø> (ø)
R/sf.R 66.03% <61.11%> (+2.83%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@agila5 agila5 changed the base branch from main to develop August 9, 2023 16:35
Base automatically changed from develop to main April 9, 2024 11:56
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.

None yet

3 participants