MNT make error message consistent in brier_score_loss#18183
Conversation
| if classes.dtype.kind not in ('O', 'U', 'S'): | ||
| # for backward compatibility, if classes are not string then | ||
| # `pos_label` will correspond to the greater label | ||
| pos_label = classes[-1] |
There was a problem hiding this comment.
It is not super clear why we accept this use case?
@jnothman Do you have the keys for this one?
There was a problem hiding this comment.
Should we open an issue about this?
There was a problem hiding this comment.
I think we should and see if this is legit. I would not block this PR for this, however :)
There was a problem hiding this comment.
I think we should at least open an issue to discuss the possibility to deprecate this behavior this behavior.
There was a problem hiding this comment.
Personally I don't think it's legit. It's likely to cause hard to detect usage issues.
| if classes.dtype.kind not in ('O', 'U', 'S'): | ||
| # for backward compatibility, if classes are not string then | ||
| # `pos_label` will correspond to the greater label | ||
| pos_label = classes[-1] |
There was a problem hiding this comment.
Should we open an issue about this?
Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
ogrisel
left a comment
There was a problem hiding this comment.
I don't really like the helper name but otherwise LGTM:
|
I renamed the function. |
Were the changes pushed to this PR? |
|
@glemaitre apparently the requested changes haven't been pushed: do you mind fixing this? Then this pull request should probably be merged. Thanks! |
|
Whoops, just pushed the missing commit. |
…8183) * MNT make error message consistent in brier_score_loss * iter * TST change error matching * PEP8 * TST check consistent error message * STY * Apply suggestions from code review Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com> * iter * cleaner * iter * iter * iter * rename function with better name * iter Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
…8183) * MNT make error message consistent in brier_score_loss * iter * TST change error matching * PEP8 * TST check consistent error message * STY * Apply suggestions from code review Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com> * iter * cleaner * iter * iter * iter * rename function with better name * iter Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
closes #15412
Make error message consistent with other metrics when
pos_labelneeds to be specified with string.Note that introducing the
_check_ambiguity_pos_labelfunction will be useful in #11096