Skip to content

FEA Add cumulative gain curve metric #18479

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Alit10
Copy link

@Alit10 Alit10 commented Sep 28, 2020

Reference Issues/PRs

Closes #10003

What does this implement/fix? Explain your changes.

Add a new metric for binary classification known as cumulative_gain_curve

Any other comments?

@Alit10 Alit10 marked this pull request as draft September 28, 2020 14:21
@Alit10 Alit10 changed the title FEA cumulative_gain_curve FEA Add cumulative gain curve metric Sep 28, 2020
@@ -968,6 +968,75 @@ def roc_curve(y_true, y_score, *, pos_label=None, sample_weight=None,

return fpr, tpr, thresholds

def cumulative_gain_curve(y_true, y_score, pos_label=None):
"""This function generates the points necessary to plot the Cumulative Gain for each ten percent of the samples
Note: This implementation is restricted to the binary classification task.
Copy link
Member

Choose a reason for hiding this comment

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

Each line should be less than 80 char, and the first sentence of the docstring should be a 1 line summary.

non-thresholded measure of decisions (as returned by
decision_function on some classifiers).
pos_label (int or str, default=None): Label considered as positive and
others are considered negative
Copy link
Member

Choose a reason for hiding this comment

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

See the docstring formatting of other functions in this module.

@marcindulak
Copy link

The original issue #10003 is about both cumulative gains and lift curves.

@Mariam-ke
Copy link
Contributor

@reshamas and I fixed Doc test errors

Base automatically changed from master to main January 22, 2021 10:53
@Alit10 Alit10 marked this pull request as ready for review March 1, 2021 17:25
Copy link
Author

@Alit10 Alit10 left a comment

Choose a reason for hiding this comment

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

Correct docstring formatting @rth

@cmarmo
Copy link
Contributor

cmarmo commented Dec 22, 2021

@Alit10 thanks for your work so far and for your patience! If you are still interested in working on this do you mind synchronizing with upstream? Thanks!

@Alit10 Alit10 closed this Dec 22, 2021
@Alit10 Alit10 reopened this Dec 22, 2021
@Alit10
Copy link
Author

Alit10 commented Dec 22, 2021

@cmarmo Hello, Done ! Is everything ok for the merge ?

@cmarmo
Copy link
Contributor

cmarmo commented Dec 23, 2021

Thanks @Alit10 ! I believe something went wrong with the synchronization.
Some files have been renamed adding _ but their old counterpart without _ are still present in your pull request.
This is also the reason why the lint is failing, messing up with some old imports.
Do you mind having a look?
@lorentzenchr is this PR concerned by the RFC issue #21718? Thanks!

@lorentzenchr
Copy link
Member

is this PR concerned by the RFC issue #21718?

Yes, it is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Discussion
Development

Successfully merging this pull request may close these issues.

Add sklearn.metrics.cumulative_gain_curve and sklearn.metrics.lift_curve
7 participants