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

Error with to_spatial_smooth when there is no pseudo-node #112

Closed
agila5 opened this issue Jan 29, 2021 · 1 comment
Closed

Error with to_spatial_smooth when there is no pseudo-node #112

agila5 opened this issue Jan 29, 2021 · 1 comment
Assignees
Labels
bug 🐛 Something isn't working
Milestone

Comments

@agila5
Copy link
Collaborator

agila5 commented Jan 29, 2021

Describe the bug
I think that the function to_spatial_smooth() should return a more informative error message in case there is no pseudo-node with the correct degree.

Reproducible example

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

# data
my_l <- 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)))
)

# sfn structure
my_sfn <- as_sfnetwork(my_l, directed = FALSE)

# convert to spatial subdivide
my_sfn <- convert(my_sfn, to_spatial_subdivision, .clean = TRUE)
#> Warning: to_spatial_subdivision assumes attributes are constant over geometries
par(mar = rep(0, 4))
plot(my_sfn)

# convert to spatial smooth
convert(my_sfn, to_spatial_smooth)
#> Error in st_sf(x, ..., agr = agr, sf_column_name = sf_column_name): no simple features geometry column present

Created on 2021-01-29 by the reprex package (v0.3.0)

It doesn't make any sense to "smooth" that network, but I think the function should simply return the original input, maybe with a warning message. Moreover, the error message is quite cryptic.

I think that the easiest solution is to add a check after the following code:

sfnetworks/R/morphers.R

Lines 550 to 555 in dd4b919

## ==========================
if (directed) {
pseudo = degree(x, mode = "in") == 1 & degree(x, mode = "out") == 1
} else {
pseudo = degree(x) == 2
}

to test if any node has the correct degree.

Expected behavior
A more informative warning/error message or return the input data.

R Session Info

sessionInfo()
#> R version 3.6.3 (2020-02-29)
#> Platform: x86_64-w64-mingw32/x64 (64-bit)
#> Running under: Windows 10 x64 (build 19041)
#> 
#> Matrix products: default
#> 
#> locale:
#> [1] LC_COLLATE=Italian_Italy.1252  LC_CTYPE=Italian_Italy.1252   
#> [3] LC_MONETARY=Italian_Italy.1252 LC_NUMERIC=C                  
#> [5] LC_TIME=Italian_Italy.1252    
#> 
#> attached base packages:
#> [1] stats     graphics  grDevices utils     datasets  methods   base     
#> 
#> other attached packages:
#> [1] sfnetworks_0.4.1 tidygraph_1.2.0  sf_0.9-7        
#> 
#> loaded via a namespace (and not attached):
#>  [1] Rcpp_1.0.6         compiler_3.6.3     pillar_1.4.7       highr_0.8         
#>  [5] class_7.3-17       tools_3.6.3        digest_0.6.27      gtable_0.3.0      
#>  [9] evaluate_0.14      lifecycle_0.2.0    tibble_3.0.6       pkgconfig_2.0.3   
#> [13] rlang_0.4.10       igraph_1.2.6       DBI_1.1.1          yaml_2.2.1        
#> [17] xfun_0.19          e1071_1.7-4        dplyr_1.0.3        stringr_1.4.0     
#> [21] knitr_1.30         sfheaders_0.4.0    generics_0.1.0     vctrs_0.3.6       
#> [25] classInt_0.4-3     grid_3.6.3         tidyselect_1.1.0   glue_1.4.2        
#> [29] R6_2.5.0           rmarkdown_2.5      ggplot2_3.3.2      purrr_0.3.4       
#> [33] tidyr_1.1.2        magrittr_2.0.1     scales_1.1.1       htmltools_0.5.0   
#> [37] ellipsis_0.3.1     units_0.6-7        assertthat_0.2.1   colorspace_2.0-0  
#> [41] KernSmooth_2.23-18 stringi_1.5.3      munsell_0.5.0      lwgeom_0.2-5      
#> [45] crayon_1.3.4

Created on 2021-01-29 by the reprex package (v0.3.0)

@agila5 agila5 added bug 🐛 Something isn't working invalid ❌ This doesn't seem right labels Jan 29, 2021
@luukvdmeer luukvdmeer removed the invalid ❌ This doesn't seem right label Jan 30, 2021
@luukvdmeer luukvdmeer self-assigned this Jan 30, 2021
@luukvdmeer luukvdmeer added this to the v0.4.2 milestone Jan 30, 2021
@luukvdmeer
Copy link
Owner

Interesting! I would even say there should be no error message at all. The smoother should just return the original network when there are no pseudo nodes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants