-
Notifications
You must be signed in to change notification settings - Fork 54
Renamed DigraphDijkstra
to DigraphShortestPaths
#730
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
base: main
Are you sure you want to change the base?
Conversation
This is looking good! The only problem now (which I didn't mention before) is that we want to preserve backwards compatibility. So, now that |
Might even make sense to do something like we do is Semigroups and mark |
DigraphDijkstra
to DigraphShortestPaths
Currently, I have renamed I had a suggestion on how to implement #735 as well:
|
This is dependent on #742, and should be updated before being merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now, could you use DeclareObsoleteSynonym
to preserve backwards compatibility? You can find some examples of this in the GAP library.
@@ -1780,7 +1780,7 @@ function(digraph, source, target) | |||
while not IsEmpty(queue) do | |||
u := Pop(queue); | |||
u := u[2]; | |||
# TODO: this has a small performance impact for DigraphDijkstraS, | |||
# TODO: this has a small performance impact for DigraphShortestPathsS, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this might be a typo that you've preserved with find-and-replace!
#570