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

Return axes #300

Merged
merged 11 commits into from
Mar 22, 2023
Merged

Return axes #300

merged 11 commits into from
Mar 22, 2023

Conversation

nwlandry
Copy link
Collaborator

Updated the plotting functions:

  • Renamed xgi_pylab.py to draw.py.
  • Removed draw_xgi_nodes became draw_nodes, draw_xgi_hyperedges became draw_hyperedges, and draw_xgi_simplices became draw_simplices.
  • Added tests for the drawing helper functions.
  • Added a pca_transform function which can be used to control the rotation of the drawing relative to the principal axes.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@codecov
Copy link

codecov bot commented Mar 21, 2023

Codecov Report

Patch coverage: 75.00% and project coverage change: +0.87 🎉

Comparison is base (6a5fb60) 83.41% compared to head (70e89e1) 84.29%.

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

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #300      +/-   ##
==========================================
+ Coverage   83.41%   84.29%   +0.87%     
==========================================
  Files          35       35              
  Lines        2732     2751      +19     
==========================================
+ Hits         2279     2319      +40     
+ Misses        453      432      -21     
Impacted Files Coverage Δ
xgi/algorithms/centrality.py 63.04% <ø> (ø)
xgi/drawing/draw.py 24.01% <46.15%> (ø)
xgi/convert.py 91.26% <100.00%> (ø)
xgi/drawing/__init__.py 100.00% <100.00%> (ø)
xgi/drawing/layout.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

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

@maximelucas
Copy link
Collaborator

Look great, thanks. The renaming is a good idea.

Do you think the codecov/patch is failing because your new PCA function is not tested?

Could you add an example of how to use the PCA function in the notebook?

@nwlandry
Copy link
Collaborator Author

All your comments should be addressed now! Added unit tests and an example in Tutorial 5.

xgi/drawing/layout.py Outdated Show resolved Hide resolved
@maximelucas
Copy link
Collaborator

maximelucas commented Mar 22, 2023

Left one more minor comment. Also the notebook for tutorial 5 has all outputs cleared.
Could you rerun it to show the outputs?

And the codecov test is still failing. I think because our codecov has slightly decrease with the PR?
Maybe we can merge and do another PR where we add tests for the draw functions as we planned anyway.

@nwlandry
Copy link
Collaborator Author

I looked up how to fix the issue and hopefully the configuration file that I added will fix the error. The issue is that even though this PR increases the coverage, it edits code that doesn't have any coverage so it treats it like I just committed a bunch of code without tests.

@maximelucas
Copy link
Collaborator

Yea that's what I understood too.
Looks great, thanks :)

@nwlandry nwlandry merged commit 7a0500f into main Mar 22, 2023
@nwlandry nwlandry deleted the return-axes branch March 22, 2023 18:46
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.

2 participants