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.

Sign up for GitHub

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

Jump to bottom

osm_get_junctions() places junctions where there are no junctions #48

Closed
Robinlovelace opened this issue Oct 9, 2020 · 13 comments
Closed

osm_get_junctions() places junctions where there are no junctions #48

Robinlovelace opened this issue Oct 9, 2020 · 13 comments

Comments

@Robinlovelace
Copy link
Contributor

No description provided.

Robinlovelace added a commit that referenced this issue Oct 9, 2020
@Robinlovelace
Add junction code, show issue #48
60612a2
@Robinlovelace
Copy link
Contributor Author

Code just pushed shows this issue:

> x = osm_main_roads(tc_data_osm)
> junctions = osm_get_junctions(x)
> plot(x$geometry, col = "grey", xlim = c(-1.59, -1.1), ylim = c(50.5, 50.8))
> plot(x$geometry, col = "grey")
> plot(junctions, add = TRUE)
> plot(x$geometry, col = "grey")
> plot(junctions, add = TRUE)

image

@Robinlovelace
Copy link
Contributor Author

Any ideas @mem48 ? Pretty close. Also heads-up @agila5 I think you've already solved this problem...

@agila5
Copy link
Contributor

agila5 commented Oct 9, 2020

At the moment I don't know, I will check it tomorrow in the afternoon!

@mem48
Copy link
Contributor

mem48 commented Oct 9, 2020

Is it where a road changes name? Or to do with enforcing segments? These can create linear junctions.

@mem48
Copy link
Contributor

mem48 commented Oct 9, 2020

Also you haven't consolidated before looking for junctions, so you will get every join between roads with different osmids

@Robinlovelace
Copy link
Contributor Author

Progress...

image

@Robinlovelace
Copy link
Contributor Author

Even better:

image

Robinlovelace added a commit that referenced this issue Oct 10, 2020
@Robinlovelace
Tidy up for #48
17ede7a
@Robinlovelace
Copy link
Contributor Author

Final result - we can close this when it gets merged:

image

@Robinlovelace
Copy link
Contributor Author

Done now, please take a look and try to reproduce this @agila5, based on a nice route network in central London: https://saferactive.github.io/trafficalmr/reference/osm_get_junctions.html

Thinking this could be of use/interest (could close) ropensci/stplanr#357 and (for reference) luukvdmeer/sfnetworks#14

Good work @mem48 now we need to demonstrate it working at scale.

@agila5
Copy link
Contributor

agila5 commented Oct 11, 2020 •
edited
Loading

Hi everyone. I don't know if I did some stupid mistake but I cannot get the same plot as in the example:

# download package
remotes::install_github("saferactive/trafficalmr")
#> Using github PAT from envvar GITHUB_PAT
#> Skipping install of 'trafficalmr' from a github remote, the SHA1 (68712ec8) has not changed since last install.
#>   Use `force = TRUE` to force installation
  
# packages
library(trafficalmr)

# example
x = osm_main_roads(tc_data_osm)
#> Loading required namespace: sf
junctions = osm_get_junctions(x)
#> Linking to GEOS 3.8.0, GDAL 3.0.4, PROJ 6.3.1
#> Splitting rnet object at the shared boundary points.
junctions2 = osm_get_junctions(x, method = "duplicated")
#> Splitting rnet object at the shared boundary points.

par(mar = rep(0, 4))
plot(x$geometry, col = "grey")
plot(junctions, add = TRUE)
plot(junctions2, add = TRUE, col = "red", cex = 0.5)


Nevertheless, I think it looks great! The only problem may be related to one junction is the north-west part of the plot, but actually that's a problem with the input data since that point is missing from both streets.

north_west <- x[c("188", "299", "374", "375", "376", "468"), ]
plot(st_geometry(north_west), col = "lightgrey", reset = FALSE)
plot(st_geometry(stplanr::line2pointsn(north_west)), add = TRUE, pch = 16)

Created on 2020-10-11 by the reprex package (v0.3.0)

Session info 
devtools::session_info()
#> - Session info ---------------------------------------------------------------
#>  setting  value                       
#>  version  R version 3.6.3 (2020-02-29)
#>  os       Windows 10 x64              
#>  system   x86_64, mingw32             
#>  ui       RTerm                       
#>  language (EN)                        
#>  collate  Italian_Italy.1252          
#>  ctype    Italian_Italy.1252          
#>  tz       Europe/Berlin               
#>  date     2020-10-11                  
#> 
#> - Packages -------------------------------------------------------------------
#>  package     * version    date       lib
#>  assertthat    0.2.1      2019-03-21 [1]
#>  backports     1.1.10     2020-09-15 [1]
#>  callr         3.4.4      2020-09-07 [1]
#>  class         7.3-17     2020-04-26 [1]
#>  classInt      0.4-3      2020-04-07 [1]
#>  cli           2.0.2      2020-02-28 [1]
#>  codetools     0.2-16     2018-12-24 [2]
#>  crayon        1.3.4      2017-09-16 [1]
#>  curl          4.3        2019-12-02 [1]
#>  DBI           1.1.0      2019-12-15 [1]
#>  desc          1.2.0      2018-05-01 [1]
#>  devtools      2.3.2      2020-09-18 [1]
#>  digest        0.6.25     2020-02-23 [1]
#>  dplyr         1.0.2      2020-08-18 [1]
#>  e1071         1.7-3      2019-11-26 [1]
#>  ellipsis      0.3.1      2020-05-15 [1]
#>  evaluate      0.14       2019-05-28 [1]
#>  fansi         0.4.1      2020-01-08 [1]
#>  foreign       0.8-75     2020-01-20 [2]
#>  fs            1.5.0      2020-07-31 [1]
#>  generics      0.0.2      2018-11-29 [1]
#>  geosphere     1.5-10     2019-05-26 [1]
#>  glue          1.4.2      2020-08-27 [1]
#>  highr         0.8        2019-03-20 [1]
#>  htmltools     0.5.0      2020-06-16 [1]
#>  httr          1.4.2      2020-07-20 [1]
#>  igraph        1.2.5      2020-03-19 [1]
#>  KernSmooth    2.23-17    2020-04-26 [1]
#>  knitr         1.30       2020-09-22 [1]
#>  lattice       0.20-41    2020-04-02 [1]
#>  lifecycle     0.2.0      2020-03-06 [1]
#>  lwgeom        0.2-5      2020-06-12 [1]
#>  magrittr      1.5        2014-11-22 [1]
#>  maptools      1.0-2      2020-08-24 [1]
#>  memoise       1.1.0      2017-04-21 [1]
#>  mime          0.9        2020-02-04 [1]
#>  pillar        1.4.6      2020-07-10 [1]
#>  pkgbuild      1.1.0      2020-07-13 [1]
#>  pkgconfig     2.0.3      2019-09-22 [1]
#>  pkgload       1.1.0      2020-05-29 [1]
#>  prettyunits   1.1.1      2020-01-24 [1]
#>  processx      3.4.4      2020-09-03 [1]
#>  ps            1.3.4      2020-08-11 [1]
#>  purrr         0.3.4      2020-04-17 [1]
#>  R6            2.4.1      2019-11-12 [1]
#>  raster        3.3-13     2020-07-17 [1]
#>  Rcpp          1.0.5      2020-07-06 [1]
#>  remotes       2.2.0      2020-07-21 [1]
#>  rgeos         0.5-5      2020-09-07 [1]
#>  rlang         0.4.7      2020-07-09 [1]
#>  rmarkdown     2.3        2020-06-18 [1]
#>  rprojroot     1.3-2      2018-01-03 [1]
#>  sessioninfo   1.1.1      2018-11-05 [1]
#>  sf          * 0.9-6      2020-09-13 [1]
#>  sp            1.4-2      2020-05-20 [1]
#>  stplanr       0.7.2      2020-09-04 [1]
#>  stringi       1.4.6      2020-02-17 [1]
#>  stringr       1.4.0      2019-02-10 [1]
#>  testthat      2.3.2      2020-03-02 [1]
#>  tibble        3.0.3.9000 2020-07-12 [1]
#>  tidyselect    1.1.0      2020-05-11 [1]
#>  trafficalmr * 0.1.0      2020-10-11 [1]
#>  units         0.6-7      2020-06-13 [1]
#>  usethis       1.6.3      2020-09-17 [1]
#>  vctrs         0.3.4      2020-08-29 [1]
#>  withr         2.3.0      2020-09-22 [1]
#>  xfun          0.17       2020-09-09 [1]
#>  xml2          1.3.2      2020-04-23 [1]
#>  yaml          2.2.1      2020-02-01 [1]
#>  source                                  
#>  CRAN (R 3.6.0)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.0)                          
#>  CRAN (R 3.6.1)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.0)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.1)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.0)                          
#>  CRAN (R 3.6.2)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.0)                          
#>  CRAN (R 3.6.1)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.0)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.2)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.0)                          
#>  CRAN (R 3.6.2)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.1)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.2)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.1)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.0)                          
#>  CRAN (R 3.6.0)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.2)                          
#>  CRAN (R 3.6.0)                          
#>  CRAN (R 3.6.3)                          
#>  Github (tidyverse/tibble@a57ad4a)       
#>  CRAN (R 3.6.3)                          
#>  Github (saferactive/trafficalmr@68712ec)
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.3)                          
#>  CRAN (R 3.6.2)                          
#> 
#> [1] C:/Users/Utente/Documents/R/win-library/3.6
#> [2] C:/Program Files/R/R-3.6.3/library

EDIT: I did a stupid mistake, sorry. I wrote duplicated instead of duplicates 😅 Now it works fine!

@Robinlovelace
Copy link
Contributor Author

Many thanks for testing @agila5. Do you think this is the 'best' implementation of a function to get the duplicated points (and since my change the junctions)? I think it's worth putting on CRAN, probably in stplanr if so. I also made the mistake re argument name. Do you think method = dups or duplicated would be better?

@agila5
Copy link
Contributor

agila5 commented Oct 12, 2020 •
edited
Loading

Do you think this is the 'best' implementation of a function to get the duplicated points (and since my change the junctions)?

I think the function is working fine! Maybe you can remove a bit of leftover code from testings (i.e. lines 152 and 157 in R/osm_clearning_functions.R) and maybe it's better working with matrices instead of data.frame(s) (line 154) but that's not so important.

Do you think method = dups or duplicated would be better?

IMO both approaches would work if you add an if clause to test that the input parameter is one of the supported values. For example:

if (! param %in% c("stplanr", "duplicates")) {
warning("The input method parameter does not correspond to "stplanr" or "duplicates", defaulting to "stplanr")
method = "stplanr"
}

If you agree, I can add a PR

@Robinlovelace
Copy link
Contributor Author

If you agree, I can add a PR

Please do if it doesn't take too much time. Many thanks @agila5 !

agila5 pushed a commit to agila5/trafficalmr that referenced this issue Oct 12, 2020
Add if clause. See discussion at the end of saferactive#48
b0de8b0
@agila5 agila5 mentioned this issue Oct 12, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@Robinlovelace @mem48 @agila5