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
DOC Typo in the error message of _binary_clf_curve #15703
DOC Typo in the error message of _binary_clf_curve #15703
Conversation
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.
Thanks for the fix.
We accept a float dtype, but we don't accept real numbers, right? Saying a value should be an integer is different from saying it should be of type int, so I think the current docstrings are correct. i.e. math domain != type |
Well @NicolasHug you can close this if you think current version is okay, but I'd argue that it's misleading. What's more, the author of the original PR has approved this PR. |
@@ -539,7 +539,7 @@ def _binary_clf_curve(y_true, y_score, pos_label=None, sample_weight=None): | |||
classes_repr = ", ".join(repr(c) for c in classes) | |||
raise ValueError("y_true takes value in {{{classes_repr}}} and " | |||
"pos_label is not specified: either make y_true " | |||
"take integer value in {{0, 1}} or {{-1, 1}} or " | |||
"take value in {{0, 1}} or {{-1, 1}} or " |
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.
single brackets?
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.
sorry what do you mean?
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.
{} instead of {{}}
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.
@NicolasHug We can't use {} here because {} has special meaning. We need to use {{}}
to output {}
.
I don't think it's misleading, but I don't mind |
I'm going to merge because the author of the original PR approved it, and Nicolas is not opposed to it. |
Typo in #15562, we support float y_true and it's tested.
ping @ogrisel @thomasjpfan