Skip to content

Code cleanup (remove dead code and move legacy files to the legacy directory)#2798

Merged
rapids-bot[bot] merged 19 commits intorapidsai:branch-22.12from
seunghwak:enh_code_cleanup
Oct 17, 2022
Merged

Code cleanup (remove dead code and move legacy files to the legacy directory)#2798
rapids-bot[bot] merged 19 commits intorapidsai:branch-22.12from
seunghwak:enh_code_cleanup

Conversation

@seunghwak
Copy link
Contributor

@seunghwak seunghwak commented Oct 12, 2022

Code clean-up before bringing bigger changes to the codebase.

Breaking because several replaced legacy functions are deleted (I guess no one is directly using them).

@seunghwak seunghwak requested review from a team as code owners October 12, 2022 17:12
@seunghwak seunghwak self-assigned this Oct 12, 2022
@seunghwak seunghwak added 3 - Ready for Review improvement Improvement / enhancement to an existing function breaking Breaking change labels Oct 12, 2022
@seunghwak seunghwak added this to the 22.12 milestone Oct 12, 2022
@ChuckHastings
Copy link
Collaborator

Some of these changes are probably premature.

Legacy jaccard/overlap is still in use by python. We also don't have an implementation for the weighted variation.

@seunghwak
Copy link
Contributor Author

Legacy jaccard/overlap is still in use by python.

I wonder why compile didn't fail in my local system. Aren't we calling these in cython/c_api?

@seunghwak
Copy link
Contributor Author

It seems like extract_subgraph_vertex is also used in python as well (isn't this induced subgraph?) But it seems like this wasn't called in cython/c_api. Do we have another mechanism to call C++ function from python (without relying on c_api or cython)?

@ChuckHastings
Copy link
Collaborator

There was, before cython.cu a mechanism that we used for calling legacy algorithms. Not everything was migrated to use python.cu, because we decided to create the C API/PLC layer and stopped migrating things to python.cu.

@codecov-commenter
Copy link

codecov-commenter commented Oct 14, 2022

Codecov Report

❗ No coverage uploaded for pull request base (branch-22.12@b3e5950). Click here to learn what that means.
Patch has no changes to coverable lines.

Additional details and impacted files
@@               Coverage Diff               @@
##             branch-22.12    #2798   +/-   ##
===============================================
  Coverage                ?   60.44%           
===============================================
  Files                   ?      111           
  Lines                   ?     6515           
  Branches                ?        0           
===============================================
  Hits                    ?     3938           
  Misses                  ?     2577           
  Partials                ?        0           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@rlratzel
Copy link
Contributor

rerun tests

reason: test timeout, try rerunning for now but may have to look into it if it persists.

@seunghwak
Copy link
Contributor Author

rerun tests

@ChuckHastings
Copy link
Collaborator

@gpucibot merge

@rapids-bot rapids-bot bot merged commit ed39ddb into rapidsai:branch-22.12 Oct 17, 2022
@seunghwak seunghwak deleted the enh_code_cleanup branch October 20, 2022 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking Breaking change improvement Improvement / enhancement to an existing function

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants