-
Notifications
You must be signed in to change notification settings - Fork 101
feat(EstimatorReport): Show permutation at different stages of a pipeline #1988
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): Show permutation at different stages of a pipeline #1988
Conversation
d5ccc35 to
cffb182
Compare
skore/src/skore/_sklearn/_estimator/feature_importance_accessor.py
Outdated
Show resolved
Hide resolved
|
Since we still don't have the permutation importance across the different reporters, there is not documentation to change in the user guide. |
cffb182 to
f2381ec
Compare
glemaitre
left a comment
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.
It looks good. I would only suggest to amend the example called plot_feature_importance.py where we can demonstate the feature. We have a Ridge model with some feature importance and we can show that we can compute with index 0 and -1.
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/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/reports/estimator/feature_importance/test_permutation_importance.py
Outdated
Show resolved
Hide resolved
skore/tests/unit/reports/estimator/feature_importance/test_permutation_importance.py
Outdated
Show resolved
Hide resolved
Now I need to figure out how to showcase the new feature in the example, hopefully in a way that doesn't disrupt the flow. |
37485da to
1d13bd8
Compare
1d13bd8 to
b90cbc4
Compare
glemaitre
left a comment
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.
Here it comes ;). Sorry for the delay.
skore/tests/unit/reports/estimator/feature_importance/test_permutation_importance.py
Show resolved
Hide resolved
skore/tests/unit/reports/estimator/feature_importance/test_permutation_importance.py
Show resolved
Hide resolved
skore/src/skore/_sklearn/_estimator/feature_importance_accessor.py
Outdated
Show resolved
Hide resolved
skore/tests/unit/reports/estimator/feature_importance/test_permutation_importance.py
Show resolved
Hide resolved
Added support for calculating permutation importance at different stages of a pipeline. One can choose to compute feature importance either at the start of the pipeline or at the end. Closes probabl-ai#1398 Supersedes probabl-ai#1888 Co-authored-by: waridrox <[email protected]>
To avoid copy-pasting all the time.
glemaitre
left a comment
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.
With those changes, you are going to cover the two missing line and it is a possible pipeline that we should be supporting.
skore/tests/unit/reports/estimator/feature_importance/test_permutation_importance.py
Outdated
Show resolved
Hide resolved
skore/tests/unit/reports/estimator/feature_importance/test_permutation_importance.py
Outdated
Show resolved
Hide resolved
skore/tests/unit/reports/estimator/feature_importance/test_permutation_importance.py
Outdated
Show resolved
Hide resolved
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
|
Now it's
|
skore/src/skore/_sklearn/_estimator/feature_importance_accessor.py
Outdated
Show resolved
Hide resolved
glemaitre
left a comment
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.
Just the last comment. Otherwise, LGTM.
Following #1988. Synchronize `pyproject.toml` and `.pre-commit-config.yaml` to let `mypy` to work outside `pre-commit`.
Added support for calculating permutation importance at different stages of a pipeline.
One can choose to compute feature importance either at the start of the pipeline or at the end.
Closes #1398
Supersedes #1888
Co-authored-by: waridrox [email protected]