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

New drawing style #369

Merged
merged 8 commits into from
May 23, 2023
Merged

New drawing style #369

merged 8 commits into from
May 23, 2023

Conversation

thomasrobiglio
Copy link
Collaborator

Related to #358, allows for new drawing style where hyperedges/simplices of different orders are draw on different layers of a 3-dimensional plot.

drawing_example

@thomasrobiglio thomasrobiglio linked an issue May 19, 2023 that may be closed by this pull request
@codecov
Copy link

codecov bot commented May 19, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.13 🎉

Comparison is base (76e9295) 90.58% compared to head (5563578) 90.72%.

❗ Current head 5563578 differs from pull request most recent head f99cb4f. Consider uploading reports for the commit f99cb4f to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #369      +/-   ##
==========================================
+ Coverage   90.58%   90.72%   +0.13%     
==========================================
  Files          41       41              
  Lines        3060     3105      +45     
==========================================
+ Hits         2772     2817      +45     
  Misses        288      288              
Impacted Files Coverage Δ
xgi/drawing/draw.py 76.02% <100.00%> (+3.63%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@thomasrobiglio thomasrobiglio marked this pull request as ready for review May 19, 2023 22:47
@nwlandry
Copy link
Collaborator

Thanks for this contribution, @thomasrobiglio! Looks great 🤩. My only question is, Is there a reason for using PolyCollection instead of scatter for the nodes? Also maybe you can open an issue on allowing 3d axes in draw_nodes(), draw_hyperedges(), etc? Overall, great work!

@thomasrobiglio
Copy link
Collaborator Author

Thank you for the comment @nwlandry!

Actually I'm not sure I understand you question - I use ax.scatter to draw the nodes at line 1417 and use Poly3DCollection at line 1409 to create the hyperedges that are then added as a 3d collection.

About opening the issues we can for sure do that. I'm a bit afraid that using draw_nodes() or draw_hyperedges() in this function would require to modify a lot the original functions while it might be easier to open issues (or do that on this PR 😄) about expanding this function to allow the same personalisation possibilities we have in the other plotting functions (e.g. edges/nodes labels or nodes' size/color from stats ...). Let me know what you think about that!

Copy link
Collaborator

@nwlandry nwlandry left a comment

Choose a reason for hiding this comment

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

Looks great to me! The comments that I was suggesting don't seem feasible.

tests/drawing/test_draw.py Outdated Show resolved Hide resolved
@maximelucas
Copy link
Collaborator

It looks very pretty @thomasrobiglio thanks :) I even showed it to my class on Friday hehe.

@leotrs leotrs merged commit e8b76a6 into xgi-org:main May 23, 2023
17 checks passed
@leotrs
Copy link
Collaborator

leotrs commented May 23, 2023

Thank you so much @thomasrobiglio !

@thomasrobiglio thomasrobiglio deleted the new_drawing_style branch June 9, 2023 08:28
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.

New drawing style
4 participants