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

Create kcomponents_weighted_graph.py #7214

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

kasturi2903
Copy link

@kasturi2903 kasturi2903 commented Jan 7, 2024

This algorithm allows to identifying kcomponents of directed graph.

Related discussion: #7106

Copy link
Contributor

@rossbar rossbar left a comment

Choose a reason for hiding this comment

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

Before getting into the details, there are some high-level organizational things that need to be taken care of before this can be considered.

The big one is the test failures, which are due to duplicate function names in the namespace. The first thing to decide from this perspective is how this functionality would be exposed to the user. I would expect the most straight-forward approach would be to add a weight kwarg to the existing k_components function. Either way, defining a new function with the same name is a non-starter.

The second important piece before this can really be reviewed is to add tests for the new behavior.

@rossbar
Copy link
Contributor

rossbar commented Mar 27, 2024

See also #1618 which is related - at the very least, the API changes in #1618 are more in line with what I'd advocate for in introducing this feature to the user (i.e. kwarg switch rather than adding/renaming functions)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

2 participants