-
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(EstimatorReport): Display the mean decrease impurity #1368
feat(EstimatorReport): Display the mean decrease impurity #1368
Conversation
475b6ef
to
3ca3f74
Compare
267eb36
to
5f3e3d0
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.
Tested, and appart from the missing example (which will be done by enriching the current one in progress I guess?), it's good for me :) !
d62096d
to
98f25b8
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.
This PR looks good to me. Only a couple of changes to speed-up the test. In terms of documentation, I think that we should be amending the example created in the linear model PR.
skore/src/skore/sklearn/_estimator/feature_importance_accessor.py
Outdated
Show resolved
Hide resolved
skore/src/skore/sklearn/_estimator/feature_importance_accessor.py
Outdated
Show resolved
Hide resolved
skore/tests/unit/sklearn/estimator/feature_importance/test_mean_decrease_impurity.py
Outdated
Show resolved
Hide resolved
skore/tests/unit/sklearn/estimator/feature_importance/test_mean_decrease_impurity.py
Outdated
Show resolved
Hide resolved
skore/tests/unit/sklearn/estimator/feature_importance/test_mean_decrease_impurity.py
Outdated
Show resolved
Hide resolved
skore/tests/unit/sklearn/estimator/feature_importance/test_mean_decrease_impurity.py
Outdated
Show resolved
Hide resolved
skore/tests/unit/sklearn/estimator/feature_importance/test_mean_decrease_impurity.py
Outdated
Show resolved
Hide resolved
98f25b8
to
eccc1e7
Compare
414ef12
to
39c86d5
Compare
2a19594
to
2a778ec
Compare
d6f9fc3
to
9c8e914
Compare
2a778ec
to
c91ef63
Compare
9c8e914
to
1c5166f
Compare
c91ef63
to
aed6a01
Compare
2ba6cff
to
c4f672e
Compare
1c5166f
to
78301d2
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.
To ease the integration of the documentation, I think this PR can be merged. I can have another look in this week to see if we should have subsequent works.
0f648dc
into
1319-featestimatorreport-display-the-feature-permutation-importance
Closes #1323
Todo: