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

Implement google_matrix and binary operators #62

Merged
merged 4 commits into from
May 3, 2023

Conversation

eriknw
Copy link
Member

@eriknw eriknw commented Apr 30, 2023

These aren't super-optimized or super-clean, but are hopefully "good enough" for a while.

@eriknw
Copy link
Member Author

eriknw commented May 1, 2023

@jim22k, test_intersection is raising, and it looks like code you added. Can you help? Note that I implemented intersection in this PR.

@jim22k
Copy link
Member

jim22k commented May 1, 2023

Other than my comment above, this all looks good to me.

@eriknw
Copy link
Member Author

eriknw commented May 2, 2023

Other than my comment above, this all looks good to me.

Thanks for reviewing! Thoughts on the failing test?

@jim22k
Copy link
Member

jim22k commented May 2, 2023

Thoughts on the failing test?

I think the line:
if os.environ.get("NETWORKX_GRAPH_CONVERT", None) != "nx-loopback":
should instead be:
if os.environ.get("NETWORKX_GRAPH_CONVERT", None) is not None:

The point of that test is to check that nx._dispatch raises a TypeError when differently typed Graph objects are passed in. But we don't need to test that during auto dispatching tests.

Can you make that change in NetworkX and see if the tests pass? If so, please add that change to your NetworkX PR. Thanks!

@eriknw
Copy link
Member Author

eriknw commented May 2, 2023

if os.environ.get("NETWORKX_GRAPH_CONVERT", None) is not None:

This doesn't work, b/c we convert networkx graphs (we always call e.g. to_graph). Should it instead be == "nx-loopback":? I still don't quite follow what's going on. Also, I think this ought to be broken out into its own test.

@jim22k
Copy link
Member

jim22k commented May 2, 2023

Oops. Try this instead:

# Only test if not performing auto convert testing of backend implementations
if os.environ.get("NETWORKX_GRAPH_CONVERT", None) is None:

The goal is to only test this if we aren't doing auto convert testing.

eriknw added a commit to eriknw/networkx that referenced this pull request May 2, 2023
@eriknw
Copy link
Member Author

eriknw commented May 3, 2023

Merging. The test failure is understood--we are waiting on NetworkX PR to be merged.

@eriknw eriknw merged commit 80ba68b into python-graphblas:main May 3, 2023
MridulS pushed a commit to networkx/networkx that referenced this pull request May 13, 2023
* Handle weights as `distance=` in testing dispatch

* fix `test_intersection`

This change was suggested here:
python-graphblas/graphblas-algorithms#62 (comment)
Alex-Markham pushed a commit to Alex-Markham/networkx that referenced this pull request Oct 13, 2023
* Handle weights as `distance=` in testing dispatch

* fix `test_intersection`

This change was suggested here:
python-graphblas/graphblas-algorithms#62 (comment)
dschult pushed a commit to BrunoBaldissera/networkx that referenced this pull request Oct 23, 2023
* Handle weights as `distance=` in testing dispatch

* fix `test_intersection`

This change was suggested here:
python-graphblas/graphblas-algorithms#62 (comment)
cvanelteren pushed a commit to cvanelteren/networkx that referenced this pull request Apr 22, 2024
* Handle weights as `distance=` in testing dispatch

* fix `test_intersection`

This change was suggested here:
python-graphblas/graphblas-algorithms#62 (comment)
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