-
-
Notifications
You must be signed in to change notification settings - Fork 25.9k
Simplified error message in get_scorer() function in sklearn.metrics.scorer.py file #11062
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
Conversation
(1) Please remember to link the issue in PR description (See the template or contributing guide) |
Can you please elaborate "irrelevant changes" ? I am not getting it. |
See the diff at https://github.com/scikit-learn/scikit-learn/pull/11062/files there are changes that are unrelated to fixing the error message. |
dea93cb
This is updated diff which consist only those changes which are related to
fixing error message. The diff which you are pointing was rejected earlier.
Thanks,
Prince
…On Thu, May 3, 2018 at 7:08 PM, Roman Yurchak ***@***.***> wrote:
Can you please elaborate "irrelevant changes" ? I am not getting it.
See the diff at #11062/
files there are changes that are unrelated to fixing the error message.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#11062 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AM1G8DXRP7PADizMAYjQ1F_h86Tk5w3Sks5tuwg8gaJpZM4TxCFC>
.
|
@princejha95 You should refer to the diff of the PR, not a single commit. |
In other words, we can only merge a pull request, not individual commits. You need to add more commits 8000 to revert the changes until the diff of the Pull Request looks right. Don't worry about individual commits too much, everything will be squashed into a single commit when merging.. |
@@ -236,8 +231,8 @@ def get_scorer(scoring): | |||
valid = False # Don't raise here to make the error message elegant | |||
if not valid: | |||
raise ValueError('%r is not a valid scoring value. ' | |||
'Valid options are %s' | |||
% (scoring, sorted(scorers))) | |||
'For valid options use sorted(SCORERS.keys())' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe should make this sklearn.metrics.SCORERS instead of just SCORERS
Ok I will be careful this time.
Thanks
Prince
…On Fri, May 4, 2018, 01:06 Roman Yurchak ***@***.***> wrote:
In other words, we can only merge a pull request, not individual commits.
You need to add more commits to revert the changes until the diff of the
Pull Request looks right. Don't worry about individual commits too much,
everything will be squashed into a single commit when merging..
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#11062 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AM1G8Nir0Hk31SytC6-JGUME2TO6jjHMks5tu1xWgaJpZM4TxCFC>
.
|
Yes I think it will be better.
Thanks
Prince
…On Fri, May 4, 2018, 05:14 Joel Nothman ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In sklearn/metrics/scorer.py
<#11062 (comment)>
:
> @@ -236,8 +231,8 @@ def get_scorer(scoring):
valid = False # Don't raise here to make the error message elegant
if not valid:
raise ValueError('%r is not a valid scoring value. '
- 'Valid options are %s'
- % (scoring, sorted(scorers)))
+ 'For valid options use sorted(SCORERS.keys())'
Maybe should make this sklearn.metrics.SCORERS instead of just SCORERS
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#11062 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AM1G8MoPKLEMADyKW-zalDnEIRYqVGP7ks5tu5Z3gaJpZM4TxCFC>
.
|
Still unrelated changes in the file. |
Reference Issues/PRs
Fixes #10712
8000What does this implement/fix? Explain your changes.
Simplified error message in get_scorer() function in sklearn.metrics.scorer.py file. Now the error message displays what the user needs to do if he gives a wrong scoring value in get_scorer() function.
Any other comments?