Skip to content

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

Draft
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

BeaMarton13
Copy link
Contributor

I used Cody for the functions.

  • By submitting this pull request, I assign the copyright of my contribution to The igraph development team.

Copy link
Member

@szhorvat szhorvat left a 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?

Copy link
Member

@ntamas ntamas left a 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.

@szhorvat szhorvat requested a review from Copilot June 22, 2025 12:47
Copy link

@Copilot Copilot AI left a 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.

Comment on lines +303 to +312
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):
Copy link
Preview

Copilot AI Jun 22, 2025

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.

Suggested change
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.

Copy link
Member

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.

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.

3 participants