Fix direction handling and incorrect EulerianDigraph requirement in FacialWalks - #931
MeikeWeiss wants to merge 8 commits into
Conversation
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (66.66%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #931 +/- ##
==========================================
- Coverage 97.45% 97.43% -0.02%
==========================================
Files 50 50
Lines 21188 21193 +5
Branches 639 639
==========================================
+ Hits 20649 20650 +1
- Misses 474 478 +4
Partials 65 65 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
james-d-mitchell
left a comment
There was a problem hiding this comment.
I'm happy with this PR, sorry if it has been sitting here waiting for a review for ages! Only one question: is it really necessary to take the symmetric closure of the graph? In many functions in Digraphs we simply assume the graph is symmetric, and return the answer as if it was. Would this be appropriate here too?
|
Thanks for the input! I changed the handling with symmetric so that it is hopefully more like in other methods. |
I fixed the issue #930 by really ignoring the direction of the edges. Additionally I fixed the same for
FacialWalksand clarified the documentation ofDualPlanarGraph. While doing this I realized thatFacialWalksrequires anEulerianDigraphas an input which is not correct.