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

Add support for nodes and edges in geographical space #357

Merged
merged 32 commits into from
Jan 29, 2024

Conversation

loreabad6
Copy link
Contributor

See #275 for details. Main issues for now are:

  1. the implementation of GeomEdgeSf in geom_edge_sf.R
  2. facetting for certain cases
  3. handling sfnetworks and sf in the NAMESPACE

Copy link
Owner

@thomasp85 thomasp85 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really good. I have made a bunch of suggestions but most are cosmetic

R/tbl_graph.R Outdated Show resolved Hide resolved
R/tbl_graph.R Outdated Show resolved Hide resolved
R/layout_sf.R Outdated Show resolved Hide resolved
R/layout_sf.R Outdated Show resolved Hide resolved
R/layout_sf.R Outdated Show resolved Hide resolved
R/layout_sf.R Outdated Show resolved Hide resolved
R/geom_edge_sf.R Outdated Show resolved Hide resolved
R/aaa.R Outdated Show resolved Hide resolved
@thomasp85
Copy link
Owner

As for your list of things, I've addressed 1) in my suggestions, 2) is unknown to me - do you have some examples of failures and I'll be happy to look into them, and 3) is also handled in the suggestions I think.

loreabad6 and others added 3 commits January 23, 2024 13:53
Co-authored-by: Thomas Lin Pedersen <thomasp85@gmail.com>
Co-authored-by: Thomas Lin Pedersen <thomasp85@gmail.com>
@loreabad6
Copy link
Contributor Author

Thank you for the suggestions, I have incorporated them.

Following up on the list:

  1. GeomEdgeSf now recognizes aesthetics but does not recognize parameters outside aes() unless edge_* is added.
ggraph(net, 'sf') +
  geom_node_sf(aes(color = central)) +
  geom_edge_sf(color = 'grey')
#> Warning in layer_sf(geom = GeomEdgeSf, data = data, mapping = mapping, stat = StatFilterSf, :
#> Ignoring unknown parameters: `colour`

### Need to write edge_* to get parameter
ggraph(net, 'sf') +
  geom_node_sf(aes(color = central)) +
  geom_edge_sf(edge_color = 'grey')

@loreabad6
Copy link
Contributor Author

And an example of a failed facetting case for point 2:

library(sfnetworks)
library(tidygraph)
library(ggraph)

net = roxel %>% 
  as_sfnetwork() %>% 
  mutate(centrality = centrality_betweenness()) %>% 
  mutate(central = ifelse(centrality > 1000, T, F)) %>% 
  activate('edges') %>% 
  mutate(azimuth = edge_azimuth(), length = edge_length())

ggraph(net, 'sf') +
  geom_node_sf(color = 'red', size = 0.05) +
  geom_edge_sf(aes(color = type)) +
  facet_graph(type ~ central) 
#> Warning: Unknown or uninitialised column: `.ggraph.index`.
#> Error in `x[i, , drop = drop]`:
#> ! Can't subset rows with `i`.
#> ✖ Logical subscript `i` must be size 1 or 701, not 0.
#> Backtrace:
#>      ▆
#>   1. ├─base::tryCatch(...)
#>   2. │ └─base (local) tryCatchList(expr, classes, parentenv, handlers)
#>   3. │   ├─base (local) tryCatchOne(...)
#>   4. │   │ └─base (local) doTryCatch(return(expr), name, parentenv, handler)
#>   5. │   └─base (local) tryCatchList(expr, names[-nh], parentenv, handlers[-nh])
#>   6. │     └─base (local) tryCatchOne(expr, names, parentenv, handlers[[1L]])
#>   7. │       └─base (local) doTryCatch(return(expr), name, parentenv, handler)
#>   8. ├─base::withCallingHandlers(...)
#>   9. ├─base::saveRDS(...)
#>  10. ├─base::do.call(...)
#>  11. ├─base (local) `<fn>`(...)
#>  12. ├─global `<fn>`(input = base::quote("cheap-stoat_reprex.R"))
#>  13. │ └─rmarkdown::render(input, quiet = TRUE, envir = globalenv(), encoding = "UTF-8")
#>  14. │   └─knitr::knit(knit_input, knit_output, envir = envir, quiet = quiet)
#>  15. │     └─knitr:::process_file(text, output)
#>  16. │       ├─knitr:::handle_error(...)
#>  17. │       │ └─base::withCallingHandlers(...)
#>  18. │       ├─base::withCallingHandlers(...)
#>  19. │       ├─knitr:::process_group(group)
#>  20. │       └─knitr:::process_group.block(group)
#>  21. │         └─knitr:::call_block(x)
#>  22. │           └─knitr:::block_exec(params)
#>  23. │             └─knitr:::eng_r(options)
#>  24. │               ├─knitr:::in_input_dir(...)
#>  25. │               │ └─knitr:::in_dir(input_dir(), expr)
#>  26. │               └─knitr (local) evaluate(...)
#>  27. │                 └─evaluate::evaluate(...)
#>  28. │                   └─evaluate:::evaluate_call(...)
#>  29. │                     ├─evaluate (local) handle(...)
#>  30. │                     │ └─base::try(f, silent = TRUE)
#>  31. │                     │   └─base::tryCatch(...)
#>  32. │                     │     └─base (local) tryCatchList(expr, classes, parentenv, handlers)
#>  33. │                     │       └─base (local) tryCatchOne(expr, names, parentenv, handlers[[1L]])
#>  34. │                     │         └─base (local) doTryCatch(return(expr), name, parentenv, handler)
#>  35. │                     ├─base::withCallingHandlers(...)
#>  36. │                     ├─base::withVisible(value_fun(ev$value, ev$visible))
#>  37. │                     └─knitr (local) value_fun(ev$value, ev$visible)
#>  38. │                       └─knitr (local) fun(x, options = options)
#>  39. │                         ├─base::withVisible(knit_print(x, ...))
#>  40. │                         ├─knitr::knit_print(x, ...)
#>  41. │                         └─knitr:::knit_print.default(x, ...)
#>  42. │                           └─evaluate (local) normal_print(x)
#>  43. │                             ├─base::print(x)
#>  44. │                             └─ggplot2:::print.ggplot(x)
#>  45. │                               ├─ggplot2::ggplot_build(x)
#>  46. │                               ├─ggraph:::ggplot_build.ggraph(x)
#>  47. │                               ├─base::NextMethod() at ggraph/R/ggraph.R:149:3
#>  48. │                               └─ggplot2:::ggplot_build.ggplot(x)
#>  49. │                                 └─layout$setup(data, plot$data, plot$plot_env)
#>  50. │                                   └─ggplot2 (local) setup(..., self = self)
#>  51. │                                     └─base::lapply(...)
#>  52. │                                       └─ggplot2 (local) FUN(X[[i]], ...)
#>  53. │                                         └─ggraph (local) map_data(...)
#>  54. │                                           └─base::lapply(...) at ggraph/R/facet_graph.R:157:9
#>  55. │                                             └─ggraph (local) FUN(X[[i]], ...)
#>  56. │                                               ├─data[data$.ggraph.index %in% nodes, , drop = FALSE] at ggraph/R/facet_graph.R:158:11
#>  57. │                                               └─sf:::`[.sf`(data, data$.ggraph.index %in% nodes, , drop = FALSE) at ggraph/R/facet_graph.R:158:11
#>  58. │                                                 ├─x[i, , drop = drop]
#>  59. │                                                 └─tibble:::`[.tbl_df`(x, i, , drop = drop)
#>  60. │                                                   └─tibble:::vectbl_as_row_index(i, x, i_arg)
#>  61. │                                                     └─tibble:::vectbl_as_row_location(i, nr, i_arg, assign, call)
#>  62. │                                                       ├─tibble:::subclass_row_index_errors(...)
#>  63. │                                                       │ └─base::withCallingHandlers(...)
#>  64. │                                                       └─vctrs::vec_as_location(...)
#>  65. └─vctrs (local) `<fn>`()
#>  66.   └─vctrs:::stop_indicator_size(...)
#>  67.     └─rlang::cnd_signal(...)

R/geom_edge_sf.R Outdated Show resolved Hide resolved
@thomasp85
Copy link
Owner

The edge param thing was an oversight when I made the last batch of suggestions.

Let me have a look at the facet thing

@thomasp85
Copy link
Owner

So, it seems everything works if geom_node_sf() follows the behaviour of the other node geoms and set data = NULL rather than data = get_sf_nodes(). Would there be any concern with this? It seems all the relevant CRS information is captured in the geometry column in the layout so get_sf_nodes() is not needed

R/geom_node_sf.R Outdated Show resolved Hide resolved
loreabad6 and others added 4 commits January 25, 2024 09:51
Co-authored-by: Thomas Lin Pedersen <thomasp85@gmail.com>
Co-authored-by: Thomas Lin Pedersen <thomasp85@gmail.com>
Co-authored-by: Thomas Lin Pedersen <thomasp85@gmail.com>
@loreabad6
Copy link
Contributor Author

So, it seems everything works if geom_node_sf() follows the behaviour of the other node geoms and set data = NULL rather than data = get_sf_nodes(). Would there be any concern with this? It seems all the relevant CRS information is captured in the geometry column in the layout so get_sf_nodes() is not needed

So I realized the main reason to include it is that when creating a ggraph only with sf nodes the projection of the data is lost, so there is distorsion since the nodes just adapt to the panel area. Maybe it could be handled with coord_sf? I will try to experiment

@thomasp85
Copy link
Owner

Yeah, I can see the CRS are not picked up with the basic layout as nodes

R/geom_node_sf.R Outdated Show resolved Hide resolved
Co-authored-by: Thomas Lin Pedersen <thomasp85@gmail.com>
@loreabad6
Copy link
Contributor Author

Thank you very much for reviewing this @thomasp85, I tested again and seems like all the issues I raised work now.

@thomasp85
Copy link
Owner

Great - can I get you to add a NEWS bullet as well

@loreabad6
Copy link
Contributor Author

Great - can I get you to add a NEWS bullet as well

Done!

@thomasp85 thomasp85 merged commit 3be75ff into thomasp85:main Jan 29, 2024
@thomasp85
Copy link
Owner

Thank you!!!

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.

2 participants