8000 Adding Fall-out, Miss rate, specificity as metrics by Bhavya1705 · Pull Request #20889 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

Adding Fall-out, Miss rate, specificity as metrics #20889

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
Closed
wants to merge 3 commits into from
Closed

Adding Fall-out, Miss rate, specificity as metrics #20889

wants to merge 3 commits into from

Conversation

Bhavya1705
Copy link
@Bhavya1705 Bhavya1705 commented Aug 30, 2021 •
edited
Loading

Reference Issues/PRs

PR: #20889

Fixes #5516 #20866 #20875. Add Code for Metrics requested.

What does this implement/fix? Explain your changes.

Adds metrics - Specificity, Sensitivity, Miss rate, Fall out.

Any other comments?

Makes changes to sklearn/metrics/_classification.py

@Bhavya1705 Bhavya1705 marked this pull request as ready for review August 30, 2021 16:58
@glemaitre
Copy link
Member

I think that you forgot to commit the changes in _classification.py and add the tests.

You should check with the contributor in #19556 to know if he/she would not be able to complete his/her PR. I think that the original PR is lacking of review and I don't think that we need a new PR apart if the contributor is not able to continue working on the problem.

@Bhavya1705
Copy link
Author
Bhavya1705 commented Sep 1, 2021 via email

@glemaitre
Copy link
Member

The _classification.py file and tests were updated in a pull request
before. I had to start a new one due to merge conflicts and changeling
check failure.

I understand this but it was not my point here. Could you:

  1. interact in the previous PR to know if the original contributor is not available to finish the work
  2. if not available, then you can start your work from is branch and resolve the conflicts (currently your PR only modify the change log)

@Bhavya1705
Copy link
Author
Bhavya1705 commented Sep 1, 2021 via email •
edited
Loading

@glemaitre
Copy link
Member

You should restart from the code in https://github.com/scikit-learn/scikit-learn/pull/19556/files
It already has been reviewed with proper function naming and following the discussion in the issue (for instance, it was discussed to not implement the sensitivity score).

So I would be inclined to review the code if it restart from the previous PR and that the changelog acknowledge all previous contributors.

@Bhavya1705
Copy link
Author

Ok Sir.

Thank You

@Pawel-Kranzberg
Copy link
Pawel-Kranzberg commented Sep 8, 2021 •
edited
Loading

Respected Sir, I had already committed the required codes in a now defunct pull request. I have updated them here also. I will also try contacting the contributor at #19556. Sorry for making this a little confusing. Also do see if my code is upto your expectations. Thank You, Regards, Bhavya Bhardwaj
…
On Wed, 1 Sep 2021, 18:49 Guillaume Lemaitre, @.***> wrote: The _classification.py file and tests were updated in a pull request before. I had to start a new one due to merge conflicts and changeling check failure. I understand this but it was not my point here. Could you: 1. interact in the previous PR to know if the original contributor is not available to finish the work 2. if not available, then you can start your work from is branch and resolve the conflicts (currently your PR only modify the change log) — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#20889 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AE4RH7QHG62TTHUOHI73QSDT7YR4ZANCNFSM5DCHR7BA .

Hi @Bhavya1705, I did the #19556 . Still waiting for that contact attempt.

@Pawel-Kranzberg
Copy link
Pawel-Kranzberg commented Sep 8, 2021 •
edited
Loading

I think that you forgot to commit the changes in _classification.py and add the tests.

You should check with the contributor in #19556 to know if he/she would not be able to complete his/her PR. I think that the original PR is lacking of review and I don't think that we need a new PR apart if the contributor is not able to continue working on the problem.

Hi @glemaitre,
#19556 contributor here (a "he"). Please let me know what would you like me to do regarding that PR. I was under the impression you wanted to close it in favour of some more optimised solution.

@glemaitre
Copy link
Member

Closing this in favor of #19556

@glemaitre glemaitre closed this Sep 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
module:metrics
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Adding Fall-out, Miss rate, specificity as metrics
3 participants
@Bhavya1705 @glemaitre @Pawel-Kranzberg
0