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

Fixing Nx and Graph/DiGraph issues #1882

Merged
merged 13 commits into from
Nov 1, 2021
Merged

Fixing Nx and Graph/DiGraph issues #1882

merged 13 commits into from
Nov 1, 2021

Conversation

BradReesWork
Copy link
Member

Fixed a number of issues with conversion of Nx directed graphs. Fix nx tests, fixed issues with using new Graph(directed=true) but checking for DiGraph

@BradReesWork BradReesWork self-assigned this Oct 13, 2021
@BradReesWork BradReesWork requested a review from a team as a code owner October 13, 2021 21:17
@BradReesWork BradReesWork added 3 - Ready for Review improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Oct 13, 2021
@BradReesWork BradReesWork added this to the 21.12 milestone Oct 13, 2021
@BradReesWork BradReesWork changed the title Branch 21.12 bug [WIP] Fixing Nx and Graph/DiGraph issues Oct 13, 2021
@BradReesWork BradReesWork changed the title [WIP] Fixing Nx and Graph/DiGraph issues Fixing Nx and Graph/DiGraph issues Oct 15, 2021
@codecov-commenter
Copy link

codecov-commenter commented Oct 15, 2021

Codecov Report

Merging #1882 (6ad9210) into branch-21.12 (5cc7799) will increase coverage by 0.18%.
The diff coverage is 96.38%.

Impacted file tree graph

@@               Coverage Diff                @@
##           branch-21.12    #1882      +/-   ##
================================================
+ Coverage         70.11%   70.30%   +0.18%     
================================================
  Files               143      143              
  Lines              8827     8863      +36     
================================================
+ Hits               6189     6231      +42     
+ Misses             2638     2632       -6     
Impacted Files Coverage Δ
...ure/graph_implementation/simpleDistributedGraph.py 23.02% <0.00%> (ø)
.../cugraph/tests/test_edge_betweenness_centrality.py 84.33% <ø> (+1.29%) ⬆️
python/cugraph/cugraph/utilities/nx_factory.py 94.23% <87.50%> (-1.61%) ⬇️
...graph/cugraph/centrality/betweenness_centrality.py 89.65% <100.00%> (-0.18%) ⬇️
...raph/structure/graph_implementation/simpleGraph.py 74.07% <100.00%> (ø)
python/cugraph/cugraph/tests/test_bfs.py 78.14% <100.00%> (ø)
python/cugraph/cugraph/tests/test_modularity.py 100.00% <100.00%> (ø)
python/cugraph/cugraph/tests/test_nx_convert.py 100.00% <100.00%> (ø)
...ython/cugraph/cugraph/community/ktruss_subgraph.py 82.35% <0.00%> (-2.95%) ⬇️
python/cugraph/cugraph/tests/test_random_walks.py 94.36% <0.00%> (-0.08%) ⬇️
... and 16 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5cc7799...6ad9210. Read the comment docs.

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.

Looks good, just had some suggestions below.

python/cugraph/cugraph/tests/test_modularity.py Outdated Show resolved Hide resolved
python/cugraph/cugraph/tests/test_modularity.py Outdated Show resolved Hide resolved
python/cugraph/cugraph/tests/test_modularity.py Outdated Show resolved Hide resolved
python/cugraph/cugraph/tests/test_modularity.py Outdated Show resolved Hide resolved
python/cugraph/cugraph/tests/test_nx_convert.py Outdated Show resolved Hide resolved
python/cugraph/cugraph/tests/test_modularity.py Outdated Show resolved Hide resolved
Copy link
Contributor

@jnke2016 jnke2016 left a comment

Choose a reason for hiding this comment

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

Overall looks good to me. just few comments

@BradReesWork
Copy link
Member Author

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 4d08cc9 into rapidsai:branch-21.12 Nov 1, 2021
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.

[FIX] analyzeClustering_modularity fails with networkx graph input
4 participants