-
Notifications
You must be signed in to change notification settings - Fork 61
ENH permutation importance #142
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
Merged
BenjaminBossan
merged 84 commits into
skops-dev:main
from
merveenoyan:feature_importance
Jan 30, 2023
Merged
Changes from all commits
Commits
Show all changes
84 commits
Select commit
Hold shift + click to select a range
18ef31b
permutation importance
merveenoyan 2ff7a5c
Update examples/plot_model_card.py
merveenoyan 5b8d4c9
Update examples/plot_model_card.py
merveenoyan ae99b89
Merge branch 'main' into feature_importance
merveenoyan e80359d
added test and got rid of pandas
merveenoyan 4ef3549
change import
merveenoyan 133cc2e
fixes
merveenoyan 1c448bc
fixes
merveenoyan 95ad03b
updated docs & more
merveenoyan 2bc714f
docs
merveenoyan b457fb9
added another test, updated docs, will add to model card rst
merveenoyan 7228e83
removed unnecessary files
merveenoyan 9471c76
added importance to model card guide
merveenoyan 48c656d
moved filepaths to tempfile
merveenoyan a514060
moved filepaths to tempfile
merveenoyan ac699c5
test windows fix
merveenoyan 76e5b0e
added types
merveenoyan baccd1b
Update skops/card/_model_card.py
merveenoyan d3d0c1c
Update skops/card/_model_card.py
merveenoyan 2489693
added matplotlib mock and mock test
merveenoyan a7ba718
fixed test
merveenoyan 1735a01
forgot to commit this lol
merveenoyan 8dd9692
change type
merveenoyan 1462ac2
Merge branch 'main' into feature_importance
merveenoyan 510d41b
added error and tests
merveenoyan 583c16d
Merge branch 'feature_importance' of github.com:merveenoyan/skops int…
merveenoyan 2828ae2
Merge branch 'main' into feature_importance
merveenoyan 97ebde9
fix for windows tests
merveenoyan ce75f12
merger
merveenoyan dd6e7aa
fix for windows tests
merveenoyan fcc16dd
fix for windows tests
merveenoyan 7fbaf89
fix for windows tests
merveenoyan a3435b8
fix for windows tests
merveenoyan abba95b
Merge branch 'main' into feature_importance
merveenoyan d0adddf
swapped with path
merveenoyan 003544f
added import_or_raise
merveenoyan 1da4747
changed pre-commit config
merveenoyan 280c602
changed pre-commit config
merveenoyan 1521444
black
merveenoyan 61d1784
fixed test
merveenoyan 35b5196
Merge branch 'main' into feature_importance
merveenoyan 5e5d6b3
trigger CI
merveenoyan 2bf6c81
minor fix after merge conflict
merveenoyan 0465163
changed test
merveenoyan 8bf590e
latest version
merveenoyan 64996fb
fix, but no idea why
adrinjalali 298f6fb
Merge branch 'main' into feature_importance
merveenoyan e03d30f
minor try
merveenoyan c9b8813
trigger ci
merveenoyan a71cdac
revert
merveenoyan 9e63a42
mypy fix
merveenoyan ec82e47
merge main
merveenoyan e33e254
fixed test and fixture
merveenoyan da185f5
Merge branch 'main' into feature_importance
merveenoyan d66be43
fix
merveenoyan b183d9c
Merge branch 'feature_importance' of github.com:merveenoyan/skops int…
merveenoyan ff9c7bf
removed redundant fixture
merveenoyan 3a93880
Merge branch 'main' into feature_importance
merveenoyan bdeabc4
trigger black
merveenoyan e999a6f
trigger black
merveenoyan c9219d5
trigger black
merveenoyan bb864a6
trigger isort
merveenoyan 9113aad
Update skops/card/_model_card.py
merveenoyan 5e80732
Update skops/card/_model_card.py
merveenoyan 322ed4d
Update skops/card/_model_card.py
merveenoyan e5afe26
Update skops/card/_model_card.py
merveenoyan 95189d8
Update skops/card/_model_card.py
merveenoyan 55b968d
Update skops/card/_model_card.py
merveenoyan 6e3fd2b
removed test, nits and more
merveenoyan 9d98003
Update skops/card/_model_card.py
merveenoyan 17e0253
Update skops/card/_model_card.py
merveenoyan 2b4df99
Merge branch 'main' into feature_importance
merveenoyan cf18588
iterated
merveenoyan 7a02cd4
added print to debug on ubuntu
merveenoyan 602b8d2
more debugging
merveenoyan c23f63f
more debugging
merveenoyan 5407351
Merge branch 'skops-dev:main' into feature_importance
merveenoyan e046f88
removed debug
merveenoyan a16d83f
removed debugging line from github workflow
merveenoyan fce9b14
removed mypy ignores
merveenoyan 1281274
Merge branch 'main' into feature_importance
merveenoyan 01a5d78
removed mypy ignores
merveenoyan 77f9f8c
removed mypy ignores
merveenoyan 7c70656
merge local
merveenoyan File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| from importlib import import_module | ||
|
|
||
|
|
||
| def import_or_raise(module, feature_name): | ||
| """Raise error if a given library is not present in the environment. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| module: str | ||
| Name of the module. | ||
|
|
||
| feature_name: str | ||
| Name of the feature module is required for. | ||
|
|
||
| Raises | ||
| ------ | ||
| ModuleNotFoundError | ||
| Is raised if a given module is not present in the environment | ||
| """ | ||
| try: | ||
| module = import_module(module) | ||
| except ImportError as e: | ||
| package = module.split(".")[0] | ||
| raise ModuleNotFoundError( | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For my understanding: Why not a simple |
||
| f"{feature_name.capitalize()} requires {package} to be installed. In order" | ||
| f" to use {feature_name}, you need to install the package in your current" | ||
| " python environment." | ||
| ) from e | ||
| return module | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| import pytest | ||
|
|
||
| from skops.utils.importutils import import_or_raise | ||
|
|
||
|
|
||
| @pytest.mark.usefixtures("matplotlib_not_installed") | ||
| def test_import_or_raise(): | ||
| with pytest.raises( | ||
| ModuleNotFoundError, | ||
| match=( | ||
| "Permutation importance requires matplotlib to be installed. In order" | ||
| " to use permutation importance, you need to install the package in" | ||
| " your current python environment." | ||
| ), | ||
| ): | ||
| import_or_raise("matplotlib", "permutation importance") |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Not sure if we should care or not, but I wonder if we should check if a file by that name already exists and maybe warn the user? If they re-create the same plot, I guess it's annoying to get a warning. But what if they already happen to have a
permutation_importances.pngin the folder? Then we would just overwrite that. Really not sure...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.
yes, we should have a flag, where by default we raise it the file exists, and only overwrite if the flag is True:
overwrite=True/False