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

Add planar layout to rustworkx #645

Open
wants to merge 47 commits into
base: main
Choose a base branch
from

Conversation

enavarro51
Copy link
Contributor

@enavarro51 enavarro51 commented Jul 26, 2022

This draft PR adds the planar layout to the graph layout methods. This code incorporates and modifies the code from the open PR #475 for the Left/Right Planarity Test. It then creates the embedding that is ultimately converted to position coordinates for displaying the graph layout.

It follows the algorithms from Ulrik Brandes: The Left-Right Planarity Test 2009 http://citeseerx.ist.psu.edu/viewdoc/summary?doi=10.1.1.217.9208
and M. Chrobak and T.H. Payne: A Linear-time Algorithm for Drawing a Planar Graph on a Grid 1989 http://citeseerx.ist.psu.edu/viewdoc/summary?doi=10.1.1.51.6677
as well as the python code used in the networkx version of planar layout.

Remaining items include

  • Additional error checking
  • Docs
  • More tests

@coveralls
Copy link

coveralls commented Jul 27, 2022

Pull Request Test Coverage Report for Build 2749795804

  • 1 of 1082 (0.09%) changed or added relevant lines in 5 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-7.6%) to 89.516%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/layout/mod.rs 0 8 0.0%
src/layout/planar.rs 0 44 0.0%
retworkx-core/src/planar/lr_planar.rs 0 445 0.0%
src/layout/embedding.rs 0 584 0.0%
Totals Coverage Status
Change from base Build 2691937771: -7.6%
Covered Lines: 12364
Relevant Lines: 13812

💛 - Coveralls

@enavarro51 enavarro51 changed the title [WIP] Add planar layout to retworkx [WIP] Add planar layout to rustworkx Aug 13, 2022
@coveralls
Copy link

coveralls commented Aug 17, 2022

Pull Request Test Coverage Report for Build 2905132284

  • 750 of 1103 (68.0%) changed or added relevant lines in 5 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-2.3%) to 94.805%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/layout/planar.rs 38 44 86.36%
src/layout/embedding.rs 481 581 82.79%
rustworkx-core/src/planar/lr_planar.rs 220 467 47.11%
Totals Coverage Status
Change from base Build 2905042897: -2.3%
Covered Lines: 13248
Relevant Lines: 13974

💛 - Coveralls

@enavarro51 enavarro51 changed the title [WIP] Add planar layout to rustworkx Add planar layout to rustworkx Aug 22, 2022
@enavarro51 enavarro51 marked this pull request as ready for review August 22, 2022 15:29
@IvanIsCoding IvanIsCoding added this to the 0.13.0 milestone Sep 21, 2022
@mtreinish mtreinish requested a review from georgios-ts October 19, 2022 18:24
Comment on lines +29 to +30
cw: Option<T>,
ccw: Option<T>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is Option the correct type here? These values will never be None. Creating an instance holding a None and later updating the weights with the correct values is redundant: store the final values right from the beginning!

@mtreinish mtreinish removed this from the 0.14.0 milestone Jan 18, 2024
@IvanIsCoding
Copy link
Collaborator

#1206 reminded me that we should revisit this PR. The new Dorogovtsev-Goltsev-Mendes generator would be a perfect example of graph that would benefit from this layout

@enavarro51
Copy link
Contributor Author

@IvanIsCoding I'll take another look at this. I haven't touched this in 2 years (It was my first rustworkx project.) I'll fix the conflicts and run some tests, especially with the new D-G-M generator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants