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

Use GenericGraph for testing shortestpaths algorithms #275

Conversation

simonschoelly
Copy link
Contributor

This PR makes the tests for the shortestpath algorithm use GenericGraph

The function in src/shortestpaths/yen.jl does currently not work with GenericGraph yet. Also a_star only kind of works with GenericGraph as this function by default takes edgetype(g) as an argument and then tries to use that type as an edge constructor. This does work for GenericEdge though. So I doubled some tests - some are run with SimpleGraph and SimpleDiGraph and some are run with GenericGraph and GenericDiGraph, but the default edgetype is replaced by SimpleEdge.

In addition, this PR fixes a bug where the has_negative_edge_cycle might not only fail on a generic graph, but return an incorrect result, as the MethodError exception was silently caught and ignored.

@simonschoelly simonschoelly self-assigned this Jul 2, 2023
@codecov
Copy link

codecov bot commented Jul 2, 2023

Codecov Report

Merging #275 (a0d5104) into master (d29e1f2) will increase coverage by 0.00%.
The diff coverage is 80.00%.

@@           Coverage Diff           @@
##           master     #275   +/-   ##
=======================================
  Coverage   97.19%   97.19%           
=======================================
  Files         114      114           
  Lines        6658     6660    +2     
=======================================
+ Hits         6471     6473    +2     
  Misses        187      187           

@simonschoelly simonschoelly mentioned this pull request Jun 29, 2023
12 tasks
@gdalle gdalle added the enhancement New feature or request label Jul 3, 2023
Copy link
Member

@gdalle gdalle left a comment

Choose a reason for hiding this comment

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

As usual, nice work! Opened an issue to track Yen

@gdalle gdalle merged commit 794beff into JuliaGraphs:master Jul 5, 2023
jwassmer pushed a commit to jwassmer/Graphs.jl that referenced this pull request Jul 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants