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

SimpleEdge comparison

Open CarloLucibello opened this issue 3 years ago • 3 comments

Implement < for SimpleEdge. This can be useful in different situations (in my case, sort and then compare two arrays of edges).

CarloLucibello avatar May 22 '22 20:05 CarloLucibello

Codecov Report

Merging #134 (37b9a02) into master (5608e49) will decrease coverage by 0.32%. The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #134      +/-   ##
==========================================
- Coverage   97.54%   97.23%   -0.32%     
==========================================
  Files         109      114       +5     
  Lines        6319     6585     +266     
==========================================
+ Hits         6164     6403     +239     
- Misses        155      182      +27     

codecov[bot] avatar May 22 '22 20:05 codecov[bot]

I fully support this. It has already come up in the past, but was not merged back then: https://github.com/sbromberger/LightGraphs.jl/pull/986/files and maybe also semi related: https://github.com/sbromberger/LightGraphs.jl/pull/1514

I am also wondering, if we should not define isless, hash and equals for all AbstractEdge.

simonschoelly avatar May 22 '22 21:05 simonschoelly

@CarloLucibello what is the status here?

simonschoelly avatar Jul 24 '22 14:07 simonschoelly

@CarloLucibello what do you think?

gdalle avatar Feb 09 '23 11:02 gdalle

Sorry I dropped this ball, the proposed changes looks fine.

CarloLucibello avatar Feb 09 '23 12:02 CarloLucibello

No worries, thanks for setting this up in the first place! Will merge as soon as the CI passes

gdalle avatar Feb 09 '23 12:02 gdalle