Graphs.jl icon indicating copy to clipboard operation
Graphs.jl copied to clipboard

Added edge-betweenness.jl to centralities

Open jwassmer opened this issue 1 year ago • 3 comments

I have added a new file edge-betweenness.jl to src/centrality/. Here I include a function to compute the edge betweenness of a graph (directed and weighted). In theory I could also add a version for MultiGraphs, but these are not in the base version of Graphs atm. My code is based on the version from networkX.

This is my first contribution, and I hope I have followed all the guidelines correctly.

Jonas

jwassmer avatar Jul 04 '23 17:07 jwassmer

Codecov Report

Merging #277 (a4016b7) into master (d3b2706) will decrease coverage by 0.02%. The diff coverage is 94.28%.

:exclamation: Current head a4016b7 differs from pull request most recent head 921d837. Consider uploading reports for the commit 921d837 to get more accurate results

@@            Coverage Diff             @@
##           master     #277      +/-   ##
==========================================
- Coverage   97.29%   97.28%   -0.02%     
==========================================
  Files         114      115       +1     
  Lines        6659     6694      +35     
==========================================
+ Hits         6479     6512      +33     
- Misses        180      182       +2     

codecov[bot] avatar Jul 04 '23 17:07 codecov[bot]

Hi Jonas, thanks for your contribution.

Code reviews here take quite a while at the moment, especially when the reviewer tries to understand the algorithm and we are also a bit understaffed, just to warn you :)

That being said, the first thing that you definitely should also implement are some tests - that is why the codecov bot is complaining so loudly. If you want to use them, we recently introduced some graph types called GenericGraph and GenericDiGraph - with these graph types we should be able to catch some errors that we might not see if we use SimpleGraph or SimpleDiGraph - this has not been changed in a lot of the existing tests yet, but here is an open PR for the other centrality algorithms if you want to look how it is done there: https://github.com/JuliaGraphs/Graphs.jl/pull/272

simonschoelly avatar Jul 04 '23 18:07 simonschoelly

You may want to add the edge_betweenness_centrality symbol to the list here so that it is made available when someone writes using Graphs.

Furthermore, you actually need to include the src/centrality/edge-betweenness.jl file, otherwise the code there is never run. That should be done in the list here.

Lastly, you also need to add test/centrality/edge-betweenness.jl file to be included in the tests that are being run when someone executes ]test Graphs]. That should be done here.

simonschoelly avatar Jul 05 '23 15:07 simonschoelly