-
Notifications
You must be signed in to change notification settings - Fork 22
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: Display the model coefficients for linear models #1339
feat: Display the model coefficients for linear models #1339
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.
We should anticipate how hard is to do the plotting with only pandas. It is to decide whether or not we should have a display.
ac49914
to
8eae80b
Compare
skore/src/skore/sklearn/_estimator/feature_importance_accessor.py
Outdated
Show resolved
Hide resolved
We have a toy example for example-driven dev at |
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 checked /examples/model_evaluation/plot_feature_importance.py and from this, it seems nice to me!
I didn't check the code nor several different models.
@thomass-dev do you know why the documentation preview does not include the new example? 🙏 Is it because the example was added after the documentation preview was generated? |
3296ad9
to
b74e296
Compare
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.
Only one comment, otherwise it's good for me!
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 just started by reviewing the example.
Now I'm thinking that .model_weights()
should return a display. The plan would be that this display has .plot()
to make the right plotting for the user, as .frame
property that return the current dataframe.
In addition, I like the current direction for the pandas style and I think that we can make it the default HTML repr of the display. One issue with the style is that it does not return a real dataframe that you can manipulate. That's why, I think that we can use it in the repr
since it is not an object that is used by the user afterwards and we still have the .frame
in case someone wants to access the real dataframe.
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.
We need to extend the test and check what happens when passing a classifier. It tells me that somehow, we might want to validate the shape of coef_
and intercept_
to something that we know how to handle and raise an error otherwise.
We have a test that passes a |
skore/src/skore/sklearn/_estimator/feature_importance_accessor.py
Outdated
Show resolved
Hide resolved
Your test does not check for multiclass. As mentioned #1339 (comment), I think that the transpose in this case will do the trick. |
Co-authored-by: Guillaume Lemaitre <[email protected]>
…-weights-for-linear-models
…r-linear-models' of https://github.com/probabl-ai/skore into 1320-featestimatorreport-display-the-feature-weights-for-linear-models
Co-authored-by: Guillaume Lemaitre <[email protected]>
…r-linear-models' of https://github.com/probabl-ai/skore into 1320-featestimatorreport-display-the-feature-weights-for-linear-models
Co-authored-by: Guillaume Lemaitre <[email protected]>
…-weights-for-linear-models
…-weights-for-linear-models
3ebfa52
to
69eb4f4
Compare
…-weights-for-linear-models
Introduces a new accessor on
EstimatorReport
,feature_importance
, which currently has one method,coefficients
, only available for estimators with acoef_
attribute (such as linear models).Demo:
Note what happens if the estimator is not a regressor (e.g. a LogisticRegression):
Close #1320
Co-authored-by: @auguste-probabl [email protected]