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

Move all new graph objects out of experimental namespace #1757

Conversation

ChuckHastings
Copy link
Collaborator

@ChuckHastings ChuckHastings commented Aug 4, 2021

Moved new graph object headers out of experimental directory.

Updated all references to new graph objects to move them into the cugraph namespace.

NOTE: There will be at least one more PR to finish this job (moving a few more files around mostly)

@ChuckHastings ChuckHastings requested review from a team as code owners August 4, 2021 21:51
@ChuckHastings ChuckHastings self-assigned this Aug 4, 2021
@ChuckHastings ChuckHastings added 3 - Ready for Review improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Aug 4, 2021
@ChuckHastings ChuckHastings added this to the 21.10 milestone Aug 4, 2021
Copy link
Contributor

@seunghwak seunghwak 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 to me, but any idea why python tests are failing?

@BradReesWork BradReesWork requested a review from aschaffer August 5, 2021 15:05
@ChuckHastings ChuckHastings requested a review from a team as a code owner August 5, 2021 15:45
@ChuckHastings
Copy link
Collaborator Author

Looks good to me, but any idea why python tests are failing?

It turns out that the SG python code is still calling the legacy katz_centrality implementation. I reverted the commit that removed katz_centrality. We'll have to wait for the python team to update the python code before we can remove the legacy implementation.

@codecov-commenter
Copy link

codecov-commenter commented Aug 5, 2021

Codecov Report

❗ No coverage uploaded for pull request base (branch-21.10@f1dffc4). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head fc7a8dc differs from pull request most recent head 77e694b. Consider uploading reports for the commit 77e694b to get more accurate results
Impacted file tree graph

@@               Coverage Diff               @@
##             branch-21.10    #1757   +/-   ##
===============================================
  Coverage                ?   59.75%           
===============================================
  Files                   ?       77           
  Lines                   ?     3523           
  Branches                ?        0           
===============================================
  Hits                    ?     2105           
  Misses                  ?     1418           
  Partials                ?        0           

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 f1dffc4...77e694b. 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.

I think it looks good, but my eyes got tired after the 40th file or so.

I'm assuming the experimental C++ tests and the EXPERIMENTAL_ prefix given to test binaries will be addressed in the next PR.

@ChuckHastings
Copy link
Collaborator Author

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 0fb1724 into rapidsai:branch-21.10 Aug 5, 2021
@ChuckHastings ChuckHastings deleted the remove_experimental_namespace_phase2 branch February 1, 2022 16:34
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.

5 participants