-
Notifications
You must be signed in to change notification settings - Fork 258
feat: expose fluid communities to python #835
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
base: develop
Are you sure you want to change the base?
feat: expose fluid communities to python #835
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.
This looks great to me @BeaMarton13, other than the small nitpicks I noted!
@ntamas, can you also have a look please?
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, apart from what Szabolcs already noted.
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.
Pull Request Overview
This PR adds support for fluid communities in the Python interface of igraph.
- Implements the fluid communities algorithm in the C and Python layers.
- Updates the igraph API (init.py) to expose the new feature.
- Includes several tests in tests/test_decomposition.py validating the expected behavior and error conditions.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
tests/test_decomposition.py | Added test cases for the fluid communities algorithm. |
src/igraph/community.py | Introduced the Python wrapper for the fluid communities method. |
src/igraph/init.py | Exposed the new fluid communities API in igraph’s public interface. |
src/_igraph/graphobject.c | Added the C binding for fluid communities. |
with self.assertRaises(Exception): | ||
g.community_fluid_communities(0) | ||
|
||
# Number of communities cannot exceed number of vertices | ||
with self.assertRaises(Exception): | ||
g.community_fluid_communities(g.vcount() + 1) | ||
|
||
# Test with disconnected graph (should raise error) | ||
g_disconnected = Graph.Full(3) + Graph.Full(3) # No connecting edge | ||
with self.assertRaises(Exception): |
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.
Consider catching a more specific exception type (e.g., ValueError) instead of the generic Exception for better clarity in testing error conditions.
with self.assertRaises(Exception): | |
g.community_fluid_communities(0) | |
# Number of communities cannot exceed number of vertices | |
with self.assertRaises(Exception): | |
g.community_fluid_communities(g.vcount() + 1) | |
# Test with disconnected graph (should raise error) | |
g_disconnected = Graph.Full(3) + Graph.Full(3) # No connecting edge | |
with self.assertRaises(Exception): | |
with self.assertRaises(ValueError): | |
g.community_fluid_communities(0) | |
# Number of communities cannot exceed number of vertices | |
with self.assertRaises(ValueError): | |
g.community_fluid_communities(g.vcount() + 1) | |
# Test with disconnected graph (should raise error) | |
g_disconnected = Graph.Full(3) + Graph.Full(3) # No connecting edge | |
with self.assertRaises(ValueError): |
Copilot uses AI. Check for mistakes.
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.
@ntamas This is actually an InternalError
. Should we assert that? How does that Python interface determine what becomes a ValueError
? Note that this is indeed an IGRAPH_EINVAL
.
I used Cody for the functions.