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.

Sign up for GitHub

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

Jump to bottom

[REVIEW] Remove unused code in UMAP. #3931

Merged
merged 1 commit into from
Jun 9, 2021
Merged

[REVIEW] Remove unused code in UMAP. #3931

merged 1 commit into from
Jun 9, 2021

Conversation

trivialfis
Copy link
Member

@trivialfis trivialfis commented Jun 2, 2021 •
edited

  • Remove unused parameters in UMAP init.
  • Remove unused function for extracting knn graph.

@trivialfis trivialfis added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Jun 2, 2021
@trivialfis trivialfis requested a review from a team as a code owner June 2, 2021 10:39
@trivialfis trivialfis requested a review from a team as a code owner June 2, 2021 11:53
@trivialfis trivialfis changed the title [REVIEW] Remove unused parameters in UMAP init. [REVIEW] Remove unused code in UMAP. Jun 2, 2021
@github-actions github-actions bot added the Cython / Python Cython or Python issue label Jun 2, 2021
@caryr35 caryr35 added this to PR-WIP in v21.08 Release via automation Jun 2, 2021
cjnolet
cjnolet requested changes Jun 2, 2021
View reviewed changes
Copy link
Member

@cjnolet cjnolet left a comment

Choose a reason for hiding this comment

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

The changes LGTM, however, I believe we will want to target this PR to branch-21.08.

v21.08 Release automation moved this from PR-WIP to PR-Needs review Jun 2, 2021
@trivialfis
Copy link
Member Author

trivialfis commented Jun 2, 2021 •
edited

Yup, it's moved into 08. I started the branch on 06 due to https://github.com/rapidsai/ops/issues/1610 .

@cjnolet cjnolet changed the base branch from branch-21.06 to branch-21.08 June 2, 2021 22:32
@trivialfis trivialfis force-pushed the enh-remove-unused-parameters branch from 77aa1fa to cb0772b Compare June 4, 2021 18:32
@trivialfis
Copy link
Member Author

Rebased onto 21.08.

@trivialfis trivialfis force-pushed the enh-remove-unused-parameters branch from cb0772b to bc5e484 Compare June 9, 2021 08:22
@trivialfis
Remove unused code in UMAP.
aedadbc 
* Remove unused parameters in init.
* Remove unused function in Python interface.
@trivialfis trivialfis force-pushed the enh-remove-unused-parameters branch from bc5e484 to aedadbc Compare June 9, 2021 18:29
cjnolet
cjnolet approved these changes Jun 9, 2021
View reviewed changes
Copy link
Member

@cjnolet cjnolet left a comment

Choose a reason for hiding this comment

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

LGTM

v21.08 Release automation moved this from PR-Needs review to PR-Reviewer approved Jun 9, 2021
@codecov-commenter
Copy link

Codecov Report

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

Impacted file tree graph

@@               Coverage Diff               @@
##             branch-21.08    #3931   +/-   ##
===============================================
  Coverage                ?   85.32%           
===============================================
  Files                   ?      230           
  Lines                   ?    18093           
  Branches                ?        0           
===============================================
  Hits                    ?    15437           
  Misses                  ?     2656           
  Partials                ?        0
Flag Coverage Δ
dask 47.90% <0.00%> (?)
non-dask 77.66% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out 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 74aaf05...aedadbc. Read the comment docs.

@cjnolet
Copy link
Member

cjnolet commented Jun 9, 2021

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 8fe1b05 into rapidsai:branch-21.08 Jun 9, 2021
v21.08 Release automation moved this from PR-Reviewer approved to Done Jun 9, 2021
@trivialfis trivialfis deleted the enh-remove-unused-parameters branch June 10, 2021 04:50
vimarsh6739 pushed a commit to vimarsh6739/cuml that referenced this pull request Oct 9, 2023
@trivialfis
Remove unused code in UMAP. (rapidsai#3931)
6be3c9a 
* Remove unused parameters in UMAP init.
* Remove unused function for extracting knn graph.

Authors:
  - Jiaming Yuan (https://github.com/trivialfis)

Approvers:
  - Corey J. Nolet (https://github.com/cjnolet)

URL: rapidsai#3931
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cjnolet cjnolet cjnolet approved these changes

Assignees
No one assigned
Labels
CUDA/C++ Cython / Python Cython or Python issue improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
No open projects
v21.08 Release
  
Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@trivialfis @codecov-commenter @cjnolet