-
Notifications
You must be signed in to change notification settings - Fork 118
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
feat: ZeroInflatedRegressor.score_samples(...)
#680
Conversation
with pytest.raises(AttributeError, match="This 'ZeroInflatedRegressor' has no attribute 'score_samples'"): | ||
zir.score_samples(X) |
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.
Wondering if this is the best error message to give to the user - automatically generated from available_if
decorator
docs/user-guide/meta-models.md
Outdated
If the underlying classifier is able to predict the _probability_ of a sample to be zero (i.e. it implements a `predict_proba` method), then the `ZeroInflatedRegressor` can be used to predict the probability of a sample being non-zero times the expected value of such sample. | ||
|
||
This quantity is sometimes called _risk estimate_ or _expected impact_, however, to adhere to scikit-learn convention, we made it accessible via the `score_samples` method. |
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.
cc: @edgBR
Is this description reasonable to you? Any input is more than welcomed
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.
@edgBR reminder ping
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.
Hi guys, the description looks clear to me. However I will add that it can be called risk only if the classifier is calibrated.
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.
I will add a warning to mention that classifier should be calibrated
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.
Did not spot anything out of the ordinary, feels cool to merge to me.
@FBruzzesi let me know if you merge this because it feels like it would be ready for a new release as well.
I was out on vacation for the weekend. Merging now 😁 |
Description
Implements
score_samples
forZeroInflatedRegressor
as discusses in #585Fixes #585
Type of change
Checklist: