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

Add entry point to tell NetworkX about nx-cugraph without importing it. #3848

Merged
merged 10 commits into from
Sep 28, 2023

Conversation

eriknw
Copy link
Contributor

@eriknw eriknw commented Sep 7, 2023

This allows NetworkX docstrings to be updated (among other things).

This will have a companion PR in NetworkX. We still need to determine (and agree) on the dict returned by this entry point, and NetworkX doesn't need to use everything I have here. We should probably add a string for "description" that gives a very short description of the backend, and maybe "url" or "homepage" or whatever so online docs can have links.

Here's how to use the entry point (Python >= 3.10) after installing it:

In [1]: from importlib.metadata import entry_points

In [2]: items = entry_points(group="networkx.plugin_info")

In [3]: [plugin] = items

In [4]: plugin.load()()
Out[4]:
{'backend_name': 'cugraph',
 'project': 'nx-cugraph',
 'package': 'nx_cugraph',
 'functions': {'betweenness_centrality',
  'edge_betweenness_centrality',
  'louvain_communities'},
 'extra_docstrings': {'betweenness_centrality': '`weight` parameter is not yet supported.',
  'edge_betweenness_centrality': '`weight` parameter is not yet supported.',
  'louvain_communities': '`threshold` and `seed` parameters are currently ignored.'},
 'extra_parameters': {'louvain_communities': {'max_level': 'Upper limit of the number of macro-iterations.'}}}

CC @rlratzel @betochimas

This allows NetworkX docstrings to be updated. This needs a companion PR in NetworkX.
@eriknw eriknw requested a review from a team as a code owner September 7, 2023 01:34
@eriknw eriknw marked this pull request as draft September 7, 2023 01:49
@eriknw
Copy link
Contributor Author

eriknw commented Sep 7, 2023

Companion networkx PR is up: networkx/networkx#6911

Comment on lines 36 to 56
"functions": {
# BEGIN: functions
"betweenness_centrality",
"edge_betweenness_centrality",
"louvain_communities",
# END: functions
},
"extra_docstrings": {
# BEGIN: extra_docstrings
"betweenness_centrality": "`weight` parameter is not yet supported.",
"edge_betweenness_centrality": "`weight` parameter is not yet supported.",
"louvain_communities": "`threshold` and `seed` parameters are currently ignored.",
# END: extra_docstrings
},
"extra_parameters": {
# BEGIN: extra_parameters
"louvain_communities": {
"max_level : int, optional": "Upper limit of the number of macro-iterations.",
},
# END: extra_parameters
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alternatively, we could organize info like:

        "functions": {
            # BEGIN: functions
            "betweenness_centrality": {
                "extra_docstring": "`weight` parameter is not yet supported.",
            },
            "edge_betweenness_centrality" : {
                "extra_docstring": "`weight` parameter is not yet supported.",
            },
            "louvain_communities": {
                "extra_docstring": "louvain_communities": "`threshold` and `seed` parameters are currently ignored.",
                "extra_parameters": {
                    "max_level : int, optional": "Upper limit of the number of macro-iterations.",
                },
            },
            # END: functions
        },

@BradReesWork BradReesWork added this to the 23.10 milestone Sep 25, 2023
@eriknw eriknw marked this pull request as ready for review September 27, 2023 02:31
Copy link
Contributor

@rlratzel rlratzel left a comment

Choose a reason for hiding this comment

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

Looked good, nothing jumped out. I had a couple of questions though.

Comment on lines +23 to +27
# from . import convert_matrix
# from .convert_matrix import *

# from . import generators
# from .generators import *
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this dead code we can remove?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is future code, yet to be born

python/nx-cugraph/_nx_cugraph/__init__.py Show resolved Hide resolved
@eriknw eriknw requested a review from a team as a code owner September 27, 2023 17:54
@rlratzel rlratzel added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Sep 27, 2023
@eriknw
Copy link
Contributor Author

eriknw commented Sep 28, 2023

@rlratzel mentioned we should update louvain to handle isolated nodes as done in #3886. I'll go ahead and make this change in this PR.

@BradReesWork
Copy link
Member

/merge

@rapids-bot rapids-bot bot merged commit f0d6333 into rapidsai:branch-23.10 Sep 28, 2023
73 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants