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 experimental source files and a few implementation headers #1763

Conversation

ChuckHastings
Copy link
Collaborator

Last phase of migrating things out of the experimental namespace. This PR moves all of the source and test files out of the experimental directory and renames the tests.

@ChuckHastings ChuckHastings requested review from a team as code owners August 6, 2021 01:39
@ChuckHastings ChuckHastings self-assigned this Aug 6, 2021
@ChuckHastings ChuckHastings added 3 - Ready for Review improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Aug 6, 2021
@ChuckHastings ChuckHastings added this to the 21.10 milestone Aug 6, 2021
@ChuckHastings ChuckHastings added this to Algorithms and Primitives in 21.10 Release Aug 6, 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. I have left one minor comment about placing PageRank tests.


###################################################################################################
# - Experimental PAGERANK tests -------------------------------------------------------------------
ConfigureTest(EXPERIMENTAL_PAGERANK_TEST experimental/pagerank_test.cpp)
ConfigureTest(PAGERANK_TEST pagerank/pagerank_test.cpp)
Copy link
Contributor

Choose a reason for hiding this comment

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

To be consistent with python, shouldn't this better go to link_analysis (after creating the directory)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Except I disagree with that python choice :-)

I'll make that change.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The src directory also uses link_analysis. Now src, tests and python all match.

@codecov-commenter
Copy link

codecov-commenter commented Aug 6, 2021

Codecov Report

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

Impacted file tree graph

@@               Coverage Diff               @@
##             branch-21.10    #1763   +/-   ##
===============================================
  Coverage                ?   59.72%           
===============================================
  Files                   ?       77           
  Lines                   ?     3521           
  Branches                ?        0           
===============================================
  Hits                    ?     2103           
  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 009b53b...1816ac0. Read the comment docs.

@ChuckHastings
Copy link
Collaborator Author

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 70b4ad3 into rapidsai:branch-21.10 Aug 6, 2021
21.10 Release automation moved this from Algorithms and Primitives to Done Aug 6, 2021
@ChuckHastings ChuckHastings deleted the remove_experimental_namespace_phase3 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
No open projects
21.10 Release
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants