Digraphs icon indicating copy to clipboard operation
Digraphs copied to clipboard

Renamed `DigraphDijkstra` to `DigraphShortestPaths`

Open devansh2605 opened this issue 9 months ago • 4 comments

https://github.com/digraphs/Digraphs/issues/570

devansh2605 avatar Feb 12 '25 15:02 devansh2605

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 DigraphDijkstra is gone, we want to introduce DigraphDijkstra as a synonym for DigraphShortestPaths. Look at how DeclareSynonym is used elsewhere in the package, and add new commits to this branch to add them to this PR.

mtorpey avatar Feb 12 '25 15:02 mtorpey

Might even make sense to do something like we do is Semigroups and mark DigraphDijkstra as obsolete:

https://github.com/semigroups/Semigroups/blob/7cc3070a8fda8d9b866b5864357dff491c133eb9/gap/obsolete.gi#L11

james-d-mitchell avatar Feb 14 '25 17:02 james-d-mitchell

Currently, I have renamed DigraphDijkstra to DigraphShortestPaths and also declared it as a synonym using DeclareSynonym. Please let me know if I should proceed with https://github.com/digraphs/Digraphs/issues/735.

I had a suggestion on how to implement https://github.com/digraphs/Digraphs/issues/735 as well:

  1. We could define a helper function DeclareDeprecatedSynonym which creates something like a wrapper that prints the warning and uses the Apply fucntion of GAP to forward the arbitrary number of arguments to the new function.
  2. We can then bind this wrapper to the name DigraphDijkstra (a string as can be seen in DeclareSynonym) using a DeclareDeprecatedSynonym. This way every call to DigraphDijkstra triggers the warning before executing DigraphShortestPaths and then can run the updated function name which is DigraphShortestPaths in this case.

devansh2605 avatar Feb 26 '25 15:02 devansh2605

This is dependent on #742, and should be updated before being merged.

james-d-mitchell avatar Apr 07 '25 08:04 james-d-mitchell