Eulerian cycles/trails for undirected graphs #232
Conversation
thchr
changed the title
Codecov Report
@@ Coverage Diff @@
## master #232 +/- ##
==========================================
+ Coverage 97.19% 97.30% +0.11%
==========================================
Files 114 115 +1
Lines 6658 6709 +51
==========================================
+ Hits 6471 6528 +57
+ Misses 187 181 -6 |
|
Gentle bump: this should be good to go (new commit today: I had forgotten to include the new tests in |
|
I just gave approval to run the tests, if they pass I'll review, if not I will haunt your nightmares forever |
|
WELL WELL |
|
I think it's a matter of dealing with exceptions in a 1.6-compatible manner |
|
Yeah, I will push a fix later today 👍 |
|
Okay, should be fixed now 👍 ! |
Oh really? |
😭 |
|
I do love a good old running gag |
|
Do you need a hand? |
|
My god; somehow I didn't commit or push the fix yesterday... The pain :( |
|
The suspense is at its highest |
|
Remaining failures on v1 and nightly appear to just be spurious |
|
#271 has been merged, so you can probably merge master into this branch and the errors will be fixed |
Oki, done 👍 |
|
Oh my, look at that! The sweet sweet perfume of passing tests |
|
Thanks for review @gdalle - latest version should address all issues, I believe. |
| error("unreachable reached") | ||
| end | ||
|
|
||
| @inline function _excludes_edge(u, w, e::AbstractEdge) |
There was a problem hiding this comment.
I think this belongs in the core utilities somewhere
|
|
||
| Returns a [Eulerian trail or cycle](https://en.wikipedia.org/wiki/Eulerian_path) through an | ||
| undirected graph `g`, starting at vertex `u`, returning a vector listing the vertices of `g` | ||
| in the order that they are traversed. If no such trail or cycle exists, throws an error. |
There was a problem hiding this comment.
How do I know whether the result will be a trail or a cycle? What does it depend on?
There was a problem hiding this comment.
Only on the degree of the starting vertex?
| error("graph is not connected: a eulerian cycle/trail does not exist") | ||
| else | ||
| # otherwise, pick whichever neighbor is not a bridge/cut-edge | ||
| bs = bridges(g) |
|
Actually, you know what? This is plenty good enough to merge. |
| function GenericGraph(elist::Vector{Graphs.SimpleGraphEdge{T}}) where {T<:Integer} | ||
| GenericGraph{T}(SimpleGraph(elist)) | ||
| end |
There was a problem hiding this comment.
@thchr I spotted this too late - I think we should not implement any method (this includes constructors) that specialize on a GenericGraph or GenericDiGraph. The reason is, that we want to use GenericGraph as a placeholder for an arbitrary AbstractGraph to verify functions that take an AbstractGraph work correctly.
There was a problem hiding this comment.
wdym by specialize on a GenericGraph?
There was a problem hiding this comment.
do you want to revert that part?
There was a problem hiding this comment.
I mean the issue I see here is, if someone writes a function like
function f(g::AbstractGraph)
T = typeof(g)
return T([Edge(1, 2)])
endthen this function might fail for an arbitrary AbstractGraph but not for a GenericGraph. I don't think it will break anything soon though, so there is no rush, but I can make a PR that works around that.
I would propose, if we need a workaround for constructors so that it will be easier to write tests, we just implement the functions generic_graph and generic_digraph - maybe I already did such a thing in one of my PRs but I don't remember exactly.
The implementation is mostly adapted from SimpleGraphs.jl.
Tagging @etiennedeg who pointed out the lack of tooling for Eulerian tours/cycles as well as the implementation in SimpleGraphs.jl.
I didn't add similar functionality for directed graphs: that could be a follow up for anyone needing it.