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.

Sign up for GitHub

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

Allow cugraph-nx to run networkx tests for nx versions 3.0, 3.1, and 3.2 #3808

Merged
merged 5 commits into from
Aug 22, 2023
Merged

Allow cugraph-nx to run networkx tests for nx versions 3.0, 3.1, and 3.2 #3808

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8b4ef6d
Allow cugraph-nx to run networkx tests for nx versions 3.0, 3.1, and 3.2
eriknw Aug 17, 2023
bba78c0
Merge branch 'branch-23.10' into cnx_test_nx_3.0_to_3.2
eriknw Aug 17, 2023
28faf52
clearer
eriknw Aug 17, 2023
544b659
Merge branch 'branch-23.10' into cnx_test_nx_3.0_to_3.2
eriknw Aug 21, 2023
a04883a
Add docstring to `on_start_tests`
eriknw Aug 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 109 additions & 8 deletions python/cugraph-nx/cugraph_nx/interface.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ class BackendInterface:
@staticmethod
def convert_from_nx(graph, *args, edge_attrs=None, weight=None, **kwargs):
if weight is not None:
# MAINT: networkx 3.0, 3.1
# For networkx 3.0 and 3.1 compatibility
if edge_attrs is not None:
raise TypeError(
Expand All @@ -38,6 +39,13 @@ def convert_to_nx(obj, *, name: str | None = None):

@staticmethod
def on_start_tests(items):
"""Modify pytest items after tests have been collected.

This is called during ``pytest_collection_modifyitems`` phase of pytest.
We use this to set `xfail` on tests we expect to fail. See:

https://docs.pytest.org/en/stable/reference/reference.html#std-hook-pytest_collection_modifyitems
"""
try:
import pytest
except ModuleNotFoundError:
Expand All @@ -51,17 +59,110 @@ def key(testpath):
return (testname, frozenset({classname, filename}))
return (testname, frozenset({filename}))

string_attribute = "unable to handle string attributes"
no_weights = "weighted implementation not currently supported"
rlratzel marked this conversation as resolved.
Show resolved Hide resolved
no_multigraph = "multigraphs not currently supported"
Copy link
Contributor

@rlratzel rlratzel Aug 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

(this is also a comment for all of on_start_tests(). No action for this PR beyond a potential FIXME if we like it.)

I'm assuming many backends will have potentially lengthy on_start_tests() implementations like this which use basically the same pattern and need frequent modification as the backends evolve to pass more tests.

I wonder if we could add a utility function to networkx which allows a backend developer to simply pass the incoming items and a YAML string and have them marked appropriately. The YAML could make it easy to define xfail messages for specific tests and nx versions, making these functions almost purely declarative and easier to read and maintain:

   from networkx.utils.backends import apply_xfails_from_yaml

   @staticmethod
   def on_start_tests(items):
      yaml = """
      nxver:
         3.2:
            test_betweenness_centrality.py:TestWeightedBetweennessCentrality.test_K5:
               weighted implementation not currently supported
      ...
      """
      apply_xfails_from_yaml(items, yaml)

The YAML could be inline like above, or in a separate file that's more discoverable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, that sounds nice; we could look into cleaning this up and making it easier for any backend to mark tests as xfail.

This seems pretty low priority for now--we'll see if I/we get frustrated by fiddling with this as we add more algorithms, and maybe we'll get motivated to make it better.


xfail = {}

from packaging.version import parse

skip = {
key("test_pajek.py:TestPajek.test_ignored_attribute"): string_attribute,
key(
"test_agraph.py:TestAGraph.test_no_warnings_raised"
): "pytest.warn(None) deprecated",
}
nxver = parse(nx.__version__)
if nxver.major == 3 and nxver.minor in {0, 1}:
# MAINT: networkx 3.0, 3.1
xfail.update(
{
key(
"test_agraph.py:TestAGraph.test_no_warnings_raised"
): "pytest.warn(None) deprecated",
key(
"test_betweenness_centrality.py:"
"TestWeightedBetweennessCentrality.test_K5"
): no_weights,
key(
"test_betweenness_centrality.py:"
"TestWeightedBetweennessCentrality.test_P3_normalized"
): no_weights,
key(
"test_betweenness_centrality.py:"
"TestWeightedBetweennessCentrality.test_P3"
): no_weights,
key(
"test_betweenness_centrality.py:"
"TestWeightedBetweennessCentrality.test_krackhardt_kite_graph"
): no_weights,
key(
"test_betweenness_centrality.py:"
"TestWeightedBetweennessCentrality."
"test_krackhardt_kite_graph_normalized"
): no_weights,
key(
"test_betweenness_centrality.py:"
"TestWeightedBetweennessCentrality."
"test_florentine_families_graph"
): no_weights,
key(
"test_betweenness_centrality.py:"
"TestWeightedBetweennessCentrality.test_les_miserables_graph"
): no_weights,
key(
"test_betweenness_centrality.py:"
"TestWeightedBetweennessCentrality.test_ladder_graph"
): no_weights,
key(
"test_betweenness_centrality.py:"
"TestWeightedBetweennessCentrality.test_G"
): no_weights,
key(
"test_betweenness_centrality.py:"
"TestWeightedBetweennessCentrality.test_G2"
): no_weights,
key(
"test_betweenness_centrality.py:"
"TestWeightedBetweennessCentrality.test_G3"
): no_multigraph,
key(
"test_betweenness_centrality.py:"
"TestWeightedBetweennessCentrality.test_G4"
): no_multigraph,
key(
"test_betweenness_centrality.py:"
"TestWeightedEdgeBetweennessCentrality.test_K5"
): no_weights,
key(
"test_betweenness_centrality.py:"
"TestWeightedEdgeBetweennessCentrality.test_C4"
): no_weights,
key(
"test_betweenness_centrality.py:"
"TestWeightedEdgeBetweennessCentrality.test_P4"
): no_weights,
key(
"test_betweenness_centrality.py:"
"TestWeightedEdgeBetweennessCentrality.test_balanced_tree"
): no_weights,
key(
"test_betweenness_centrality.py:"
"TestWeightedEdgeBetweennessCentrality.test_weighted_graph"
): no_weights,
key(
"test_betweenness_centrality.py:"
"TestWeightedEdgeBetweennessCentrality."
"test_normalized_weighted_graph"
): no_weights,
key(
"test_betweenness_centrality.py:"
"TestWeightedEdgeBetweennessCentrality.test_weighted_multigraph"
): no_multigraph,
key(
"test_betweenness_centrality.py:"
"TestWeightedEdgeBetweennessCentrality."
"test_normalized_weighted_multigraph"
): no_multigraph,
}
)
for item in items:
kset = set(item.keywords)
for (test_name, keywords), reason in skip.items():
for (test_name, keywords), reason in xfail.items():
if item.name == test_name and keywords.issubset(kset):
item.add_marker(pytest.mark.xfail(reason=reason))

Expand Down
1 change: 1 addition & 0 deletions python/cugraph-nx/pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ test = [
"pytest",
"pytest-benchmark",
"pytest-mpl",
"packaging >=21",
]

[project.urls]
Expand Down
8 changes: 6 additions & 2 deletions python/cugraph-nx/run_nx_tests.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,9 @@
#
# Coverage of `cugraph_nx.algorithms` is reported and is a good sanity check that algorithms run.

# NETWORKX_GRAPH_CONVERT=cugraph NETWORKX_BACKEND_TEST_EXHAUSTIVE=True pytest --pyargs networkx "$@"
NETWORKX_TEST_BACKEND=cugraph NETWORKX_TEST_FALLBACK_TO_NX=True pytest --pyargs networkx --cov=cugraph_nx/algorithms --cov-report term-missing --no-cov-on-fail "$@"
NETWORKX_GRAPH_CONVERT=cugraph NETWORKX_BACKEND_TEST_EXHAUSTIVE=True \
NETWORKX_TEST_BACKEND=cugraph NETWORKX_TEST_FALLBACK_TO_NX=True \
pytest --pyargs networkx \
--cov=cugraph_nx/algorithms \
--cov-report term-missing --no-cov-on-fail \
"$@"