-
Notifications
You must be signed in to change notification settings - Fork 164
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
Add reverse inplace function for digraph #853
Merged
mergify
merged 11 commits into
Qiskit:main
from
matanco64:add_reverse_inplace_function_for_digraph
May 10, 2023
Merged
Changes from 7 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
2835be4
added a reverse_inplace function in digraph,
matanco64 e0ab72d
added python tests for the reverse_inplace function.
matanco64 175d57f
ran rust fmt and clippy, also added more detailed documentation
matanco64 082490a
rename reverse_inplace to reverse
matanco64 d89858a
change excepts to unwraps (If this fails is because of PyO3. It panic…
matanco64 805f7d6
added tests for empty graph and graph with node removed in the middle
matanco64 7babc2d
added interface signature for IDEs
matanco64 6ff3236
ran cargo fmt
matanco64 07c5f40
Merge branch 'main' into add_reverse_inplace_function_for_digraph
matanco64 6fdf5b8
Merge remote-tracking branch 'origin/main' into add_reverse_inplace_f…
mtreinish 03dc8e2
Fix doc syntax
mtreinish File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Two small test cases that I think would be good to add is one with an empty graph. The other is for a graph where there is an edge removed in the middle (which will leave a hole in the edge indices). I don't expect there are any problems with the method's behavior with these, but they're common edge cases that are good to verify (because a lot of other methods have had issues in the past with both).
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.
Hey Sorry for the late response and update, I've added the tests