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

Allow empty edges #565

Merged
merged 15 commits into from
Aug 20, 2024
Merged

Allow empty edges #565

merged 15 commits into from
Aug 20, 2024

Conversation

nwlandry
Copy link
Collaborator

@nwlandry nwlandry commented Jul 26, 2024

This PR enables empty edges. This has long been an inconsistency, since add_edges_from allowed empty edges and, for example, dual could create empty edges. This PR does the following:

  • removes the exception raised for empty edges from the add_edge methods in Hypergraph and DiHypergraph and from add_simplex in SimplicialComplex.
  • adds an allow_empty keyword in the remove_node methods to handle discarding empty edges created from weakly removing nodes.
  • makes all the keywords in remove_node accessible as options in the remove_nodes_from methods for consistency.

This fixes #403.

Copy link

codecov bot commented Jul 26, 2024

Codecov Report

Attention: Patch coverage is 94.73684% with 1 line in your changes missing coverage. Please review.

Project coverage is 92.82%. Comparing base (0475563) to head (1f22702).
Report is 53 commits behind head on main.

Files with missing lines Patch % Lines
xgi/core/views.py 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##           main     #565       +/-   ##
=========================================
+ Coverage      0   92.82%   +92.82%     
=========================================
  Files         0       59       +59     
  Lines         0     4348     +4348     
=========================================
+ Hits          0     4036     +4036     
- Misses        0      312      +312     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@nwlandry
Copy link
Collaborator Author

@tlarock and @thomasrobiglio --- let me know if you have the bandwidth to review this. The URL workflow fails, but this is corrected in an already-merged PR.

@thomasrobiglio
Copy link
Collaborator

Hi @nwlandry, thank you for this. I am offline for these two weeks, I will review once I'm back.

@nwlandry
Copy link
Collaborator Author

nwlandry commented Aug 1, 2024

Enjoy your vacation! That sounds good.

Copy link
Collaborator

@tlarock tlarock left a comment

Choose a reason for hiding this comment

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

Need to make sure docstrings match function signatures re: default behaviors. Once that is done, good to go.

xgi/core/dihypergraph.py Outdated Show resolved Hide resolved
xgi/core/dihypergraph.py Outdated Show resolved Hide resolved
xgi/core/hypergraph.py Show resolved Hide resolved
@nwlandry
Copy link
Collaborator Author

@thomasrobiglio --- are you still able to review this? No worries if not!

@nwlandry nwlandry merged commit 5d67d6b into main Aug 20, 2024
24 checks passed
@nwlandry nwlandry deleted the empty-edges branch August 20, 2024 18:06
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.

Consistency in dealing with empty edges
3 participants