Skip to content

feat: add strict parameter to get_condorcet_cycles (#327)#330

Merged
peterrrock2 merged 4 commits into
mggg:3.4.0from
hayan-gh:issue-327-condorcet-strict-parameter
Feb 27, 2026
Merged

feat: add strict parameter to get_condorcet_cycles (#327)#330
peterrrock2 merged 4 commits into
mggg:3.4.0from
hayan-gh:issue-327-condorcet-strict-parameter

Conversation

@hayan-gh
Copy link
Copy Markdown
Contributor

@hayan-gh hayan-gh commented Feb 26, 2026

Fixes #327

Copy link
Copy Markdown
Collaborator

@peterrrock2 peterrrock2 left a comment

Choose a reason for hiding this comment

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

Thanks for working on this! A couple of small things in here. If you like them, you can just accept the suggestion.

Also, can you please add some tests for these to the test suite?

strict_edges.append((u, v, d))

graph_to_use = nx.DiGraph()
graph_to_use.add_nodes_from(self.pairwise_graph.nodes)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
graph_to_use.add_nodes_from(self.pairwise_graph.nodes)

Since we are looking for cycles, we don't need to worry about isolated nodes, and the add_edges_from method will add the nodes that are missing anyway.

Comment on lines +366 to +368
for u, v, d in self.pairwise_graph.edges(data=True):
if d.get("weight", 0) > 0:
strict_edges.append((u, v, d))
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
for u, v, d in self.pairwise_graph.edges(data=True):
if d.get("weight", 0) > 0:
strict_edges.append((u, v, d))
for u, v, data in self.pairwise_graph.edges(data=True):
if data.get("weight", 0) > 0:
strict_edges.append((u, v, data))

personal preference: more explicit variable names

Comment on lines +351 to +357
Returns a list of condorcet cycles in the graph.

A cycle is defined as any cycle of length greater than 2.

Args:
strict (bool, optional): If True, only considers strict wins (weight > 0).
If False, includes ties (weak condorcet cycles). Defaults to False.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you for styling this!

@hayan-gh hayan-gh requested a review from peterrrock2 February 27, 2026 08:17
Copy link
Copy Markdown
Collaborator

@peterrrock2 peterrrock2 left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks!

@peterrrock2 peterrrock2 merged commit d2da6e0 into mggg:3.4.0 Feb 27, 2026
3 checks passed
@peterrrock2 peterrrock2 mentioned this pull request Apr 9, 2026
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