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.

Sign up for GitHub

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

Jump to bottom

Check if nodes exist on add_edge methods #862

Merged
merged 18 commits into from
May 12, 2023
Merged

Check if nodes exist on add_edge methods #862

merged 18 commits into from
May 12, 2023

Conversation

IvanIsCoding
Copy link
Collaborator

Close #855

This checks if the nodes exist and if they do not, it raises an IndexError instead of panicking

@coveralls
Copy link

coveralls commented Apr 14, 2023 •
edited
Loading

Pull Request Test Coverage Report for Build 4963044728

  • 26 of 26 (100.0%) changed or added relevant lines in 4 files are covered.
  • 2 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.004%) to 96.933%
Files with Coverage Reduction New Missed Lines %
src/shortest_path/all_pairs_bellman_ford.rs 1 99.44%
src/shortest_path/all_pairs_dijkstra.rs 1 98.54%
Totals Coverage Status
Change from base Build 4961110683: -0.004%
Covered Lines: 14476
Relevant Lines: 14934
💛 - Coveralls

@mtreinish mtreinish added this to the 0.13.0 milestone May 10, 2023
mtreinish
mtreinish reviewed May 10, 2023
View reviewed changes
Copy link
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this. I'm wondering though if we have the same underlying issue with all the edge addition methods (except for the extend* ones).

src/digraph.rs Show resolved Hide resolved
releasenotes/notes/handle-non-existent-edge-15d70cfe60c89ac2.yaml Outdated Show resolved Hide resolved
releasenotes/notes/handle-non-existent-edge-15d70cfe60c89ac2.yaml Outdated Show resolved Hide resolved
IvanIsCoding and others added 11 commits May 11, 2023 21:41
@IvanIsCoding @mtreinish
Apply suggestions from code review
899bff2 
Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
@Eric-Arellano @mtreinish @IvanIsCoding
Fix docs to work with Sphinx Theme 1.11 (Qiskit#867)
43a6058 
* Fix docs to work with Sphinx Theme 1.11

* Update docs/source/_templates/sidebar.html

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
@Eric-Arellano @mtreinish @IvanIsCoding
Turn off CI for forks (Qiskit#868)
9fddde0 
Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
@binh-vu @mtreinish @IvanIsCoding
Fix pickle/deepcopy not preserve original edge indices (Qiskit#589)
4c9d26e 
* fix issue Qiskit#585 that pickling graph & digraph do not preserve original edge index

* fix clippy lints - collapsible_else_if

* Simplify logic in __setstate__

* Add release note

* Fix lint

---------

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
@dependabot @mtreinish @IvanIsCoding
Bump serde from 1.0.160 to 1.0.162 (Qiskit#863)
e4d0b47 
Bumps [serde](https://github.com/serde-rs/serde) from 1.0.160 to 1.0.162.
- [Release notes](https://github.com/serde-rs/serde/releases)
- [Commits](serde-rs/serde@v1.0.160...1.0.162)

---
updated-dependencies:
- dependency-name: serde
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
@matanco64 @mtreinish @IvanIsCoding
Add reverse inplace function for digraph (Qiskit#853)
636b81e 
* added a reverse_inplace function in digraph,

the function reverses the direction of the edges in the digraph
implemented by switching the indices of the nodes in an edge.

* added python tests for the reverse_inplace function.

testing a simple case and a case for a large graph.

* ran rust fmt and clippy, also added more detailed documentation

* rename reverse_inplace to reverse

* change excepts to unwraps (If this fails is because of PyO3. It panics and there is not much point in printing a message)

* added tests for empty graph and graph with node removed in the middle

* added interface signature for IDEs

* ran cargo fmt

* Fix doc syntax

---------

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
@dependabot @IvanIsCoding
Bump serde from 1.0.162 to 1.0.163 (Qiskit#869)
0e77bf2 
Bumps [serde](https://github.com/serde-rs/serde) from 1.0.162 to 1.0.163.
- [Release notes](https://github.com/serde-rs/serde/releases)
- [Commits](serde-rs/serde@v1.0.162...v1.0.163)

---
updated-dependencies:
- dependency-name: serde
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@IvanIsCoding
Extend fixes to add_edges_from and add_edges_from_no_data
bd5bcff
@IvanIsCoding
Merge remote-tracking branch 'upstream/main' into fix-add-edge-panic
471b998
@IvanIsCoding
Lower amount of nodes in test
3c352b4
@IvanIsCoding
Merge branch 'main' into fix-add-edge-panic
0cbeb35
mtreinish
mtreinish approved these changes May 12, 2023
View reviewed changes
Copy link
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the quick update

@mtreinish mtreinish added this pull request to the merge queue May 12, 2023
@mtreinish mtreinish removed this pull request from the merge queue due to a manual request May 12, 2023
@mtreinish mtreinish added the automerge Queue a approved PR for merging label May 12, 2023
@mergify mergify bot merged commit e538391 into Qiskit:main May 12, 2023
IvanIsCoding added a commit to IvanIsCoding/rustworkx that referenced this pull request May 26, 2023
@IvanIsCoding @mtreinish
@Eric-Arellano @binh-vu @dependabot @matanco64
Check if nodes exist on add_edge methods (Qiskit#862)
71890f2 
* Add tests from the example

* Fix bug

* Fix tests

* Add release note

* Update release note

* Apply suggestions from code review

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>

* Fix docs to work with Sphinx Theme 1.11 (Qiskit#867)

* Fix docs to work with Sphinx Theme 1.11

* Update docs/source/_templates/sidebar.html

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>

* Turn off CI for forks (Qiskit#868)

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>

* Fix pickle/deepcopy not preserve original edge indices (Qiskit#589)

* fix issue Qiskit#585 that pickling graph & digraph do not preserve original edge index

* fix clippy lints - collapsible_else_if

* Simplify logic in __setstate__

* Add release note

* Fix lint

---------

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>

* Bump serde from 1.0.160 to 1.0.162 (Qiskit#863)

Bumps [serde](https://github.com/serde-rs/serde) from 1.0.160 to 1.0.162.
- [Release notes](https://github.com/serde-rs/serde/releases)
- [Commits](serde-rs/serde@v1.0.160...1.0.162)

---
updated-dependencies:
- dependency-name: serde
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Matthew Treinish <mtreinish@kortar.org>

* Add reverse inplace function for digraph (Qiskit#853)

* added a reverse_inplace function in digraph,

the function reverses the direction of the edges in the digraph
implemented by switching the indices of the nodes in an edge.

* added python tests for the reverse_inplace function.

testing a simple case and a case for a large graph.

* ran rust fmt and clippy, also added more detailed documentation

* rename reverse_inplace to reverse

* change excepts to unwraps (If this fails is because of PyO3. It panics and there is not much point in printing a message)

* added tests for empty graph and graph with node removed in the middle

* added interface signature for IDEs

* ran cargo fmt

* Fix doc syntax

---------

Co-authored-by: Matthew Treinish <mtreinish@kortar.org>

* Bump serde from 1.0.162 to 1.0.163 (Qiskit#869)

Bumps [serde](https://github.com/serde-rs/serde) from 1.0.162 to 1.0.163.
- [Release notes](https://github.com/serde-rs/serde/releases)
- [Commits](serde-rs/serde@v1.0.162...v1.0.163)

---
updated-dependencies:
- dependency-name: serde
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Extend fixes to add_edges_from and add_edges_from_no_data

* Lower amount of nodes in test

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Matthew Treinish <mtreinish@kortar.org>
Co-authored-by: Eric Arellano <14852634+Eric-Arellano@users.noreply.github.com>
Co-authored-by: Binh Vu <binh-vu@users.noreply.github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: matanco64 <38103422+matanco64@users.noreply.github.com>
@IvanIsCoding IvanIsCoding deleted the fix-add-edge-panic branch May 28, 2023 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mtreinish mtreinish mtreinish approved these changes

Assignees
No one assigned
Labels
automerge Queue a approved PR for merging
Projects
None yet
Milestone
0.13.0
Development

Successfully merging this pull request may close these issues.

PyDiGraph.add_edge() and PyGraph.add_edge panic if an endpoint doesn't exist
6 participants
@IvanIsCoding @coveralls @mtreinish @Eric-Arellano @binh-vu @matanco64