-
Notifications
You must be signed in to change notification settings - Fork 34
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
Drawing hull as an option in draw_hyperedges #491
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #491 +/- ##
==========================================
+ Coverage 91.89% 92.04% +0.14%
==========================================
Files 60 60
Lines 4441 4372 -69
==========================================
- Hits 4081 4024 -57
+ Misses 360 348 -12
☔ View full report in Codecov by Sentry. |
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.
Looks good @maximelucas, thank you.
Minor fixes:
- remove ref. in the API xgi.drawing.draw.rst
- I think the tutorial Tutorial 7 - Convex hulls hypergraph plotting.ipynb is better to put it in the in depth (as a separate one) or at the end of In Depth 2 - Drawing hyperedges.ipynb
- About the nodes' size change I think it is nice, looks cleaner this way. Maybe we should add a line somewhere in the docs telling the thing for the labels (that the user has to change the nodes' size manually)... or we can implement a thing that automatically draws the labels prop to node size (it's like that in graph tool), but I think in a new PR
Good ideas Thomas! Should be all good now. For 3) I added a note in the docstrings. At first I thought we could just do the following to change it automatically like you said if node_labels and node_size is None:
node_size = 15 but by default |
draw_hypergraph_hull
was last function to make consistent #477. I noticed the edge-plotting part was doing exactly the same asdraw_hyperedges
except for the few lines drawing the hulls. So I:hull=False
andradius
argument todraw_hyperedges
, which needed just 10 more lines of code to work. So everything works out of the box becausedraw_hyperedges
works.draw
so we can now doxgi.draw(H, hull=True)
.draw_hypergraph_hull
and_draw_hull
as they are not used anymore._color_arg_to_dict
as not used anymoreThis closes #477 and #388. Related to #404.