-
Notifications
You must be signed in to change notification settings - Fork 145
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 Dorogovtsev-Goltsev-Mendes graph generator #1206
Conversation
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.
Overall Rust code looks good. I did leave some comments about the Python tests though. I am not an expert on this type of graph but I think we can add some code to ensure at least it keeps the core properties
for n in range(0, 6): | ||
graph = rustworkx.generators.dorogovtsev_goltsev_mendes_graph(n) | ||
self.assertEqual(len(graph), (3**n + 3) // 2) | ||
self.assertEqual(len(graph.edges()), 3**n) |
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 it is also worth checking that the graph you generate is planar. We have https://www.rustworkx.org/apiref/rustworkx.is_planar.html
Pull Request Test Coverage Report for Build 9330743739Details
💛 - Coveralls |
Thanks @IvanIsCoding for the review, I made the changes to the tests. |
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.
LGTM, thanks for addressing the feedback quickly! Hopefully we release it in 0.15 soon-ish
Related to #150.
This adds generator functions for Dorogovtsev-Goltsev-Mendes graphs to both rustworkx and rustworkx-core.
First contribution, feedback is welcome.