Skip to content
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

466 add feature importance cli #468

Merged
merged 11 commits into from
Dec 4, 2024
Merged

Conversation

msorvoja
Copy link
Collaborator

Fixes #466

Copy link
Collaborator

@nmaarnio nmaarnio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple comments, see below.

Besides those, I looked at the toolkit function and a couple things could be changed there as part of this PR:

  • Renamex_test-> X and change description, does not need to be testing data. It can be the same exact data that is fed into any Sklearn model
  • Rename y_test -> y and change description, does not need to be testing data
  • Change n_repeats default to 10? It is 5 in Sklearn, if we want higher maybe 10 is enough
  • Add check that feature_names length matches the dimensions of X (X.shape[1] I think?)
  • Maybe we can remove the multiplication by 100 for the importance means – I think the range [0,1] where they reside by default is pretty intuitive and used in other phases of MPM / EIS Toolkit in general

eis_toolkit/cli.py Outdated Show resolved Hide resolved
eis_toolkit/cli.py Outdated Show resolved Hide resolved
@nmaarnio nmaarnio merged commit 5ca25b3 into master Dec 4, 2024
3 of 4 checks passed
@nmaarnio nmaarnio deleted the 466-add-feature-importance-cli branch December 4, 2024 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add feature importance CLI
2 participants