ENH add importance_getter to RFE* and SelectFromModel #15361

venkyyuvy · 2019-10-25T12:06:43Z

Reference Issues/PRs

Fixes #15312

What does this implement/fix? Explain your changes.

Any other comments?

Update upstream

Scoring clf looe

jnothman · 2019-10-26T12:26:25Z

Thanks. We could also consider the user specifying an attribute name/path (implemented with attrgetter)

sklearn/feature_selection/rfe.py

venkyyuvy · 2019-10-27T17:45:22Z

Thanks. We could also consider the user specifying an attribute name/path (implemented with attrgetter)

Thanks for the pointers. I have added these options now.

jnothman

Do you think it is clearer if we don't add the attribute path support, and just handle 'auto' and callable? Maybe I shouldn't have suggested the additional option...

sklearn/feature_selection/rfe.py

venkyyuvy · 2019-10-28T08:45:44Z

Do you think it is clearer if we don't add the attribute path support, and just handle 'auto' and callable? Maybe I shouldn't have suggested the additional option...

Nop. I think including param/path support would be user-friendly.
Previously, I had misunderstood you suggestion as to get the input as attrgetter.

…t-learn into param_importance

sklearn/feature_selection/_rfe.py

glemaitre · 2020-05-13T12:35:30Z

I will have a look at it. I am running behind all my late review ;P

doc/whats_new/v0.23.rst

sklearn/feature_selection/_rfe.py

glemaitre

OK a couple of comments. Basically, this is fine but I think that we can avoid some code duplicate.

sklearn/feature_selection/_from_model.py

sklearn/feature_selection/_rfe.py

sklearn/feature_selection/_base.py

sklearn/feature_selection/_rfe.py

glemaitre · 2020-05-13T13:35:13Z

sklearn/feature_selection/tests/test_rfe.py

+def test_w_target_transform(importance_getter, model):
+    n_features = 10
+    X, y = make_friedman1(n_samples=50, n_features=10, random_state=0)
+    estimator = SVR(kernel="linear")


yes LinearSVR sorry for the wrong pointer.

sklearn/feature_selection/tests/test_rfe.py

glemaitre · 2020-05-13T13:37:01Z

sklearn/feature_selection/_rfe.py

@@ -8,6 +8,7 @@

 import numpy as np
 from joblib import Parallel, delayed, effective_n_jobs
+from operator import attrgetter


Once the refactoring done, you will not need this import ;)

sklearn/feature_selection/tests/test_rfe.py

removing attrgetter in docstring

glemaitre

Almost there. Only nitpicks. I would add a bit of documentation and I think that we are good to go.

sklearn/feature_selection/_base.py

sklearn/feature_selection/tests/test_rfe.py

renaming expected_features to expected_n_features

venkyyuvy · 2020-05-15T05:21:33Z

Do you think, we need to add the .. versionadded:: 0.24 tag for this param?

Also, would it be helpful if we add one example, which uses the importance_getter in the docstring?

glemaitre · 2020-05-19T11:36:10Z

Do you think, we need to add the .. versionadded:: 0.24 tag for this param?

Good remark. Yes, we need it.

Also, would it be helpful if we add one example, which uses the importance_getter in the docstring?

We can let it for now. I think that with the user guide it should be enough.

glemaitre

Just 2 quick changes and the version added and we are good to merge.

sklearn/feature_selection/_from_model.py

venkyyuvy · 2020-05-20T03:31:32Z

We can let it for now. I think that with the user guide it should be enough.

Even the user guide is not updated as well.
Guess we need to update that?

Also there is no reference for user guide in documentation

sklearn/feature_selection/_from_model.py

jnothman · 2020-05-20T06:15:40Z

Thanks @venkyyuvy!! Nice work!

venkyyuvy and others added 12 commits August 26, 2019 14:20

Merge pull request #1 from scikit-learn/master

81e2edf

Update upstream

handle_clf_ridgeGCV

0f1d1ef

included test case

494f1e0

undo_lambda_rm

4b103f5

crtd_commet

2eb63b9

check_y

f06a0ed

include_comp_proba

fd145bb

fix_format

8ba98de

Merge pull request #2 from venkyyuvy/scoring_clf_looe

fee686b

Scoring clf looe

adding importance_getter param

92960c6

forwarding

c070504

reversing_changes

4a7c73e

jnothman reviewed Oct 26, 2019

View reviewed changes

sklearn/feature_selection/rfe.py Outdated Show resolved Hide resolved

sklearn/feature_selection/rfe.py Outdated Show resolved Hide resolved

sklearn/feature_selection/rfe.py Outdated Show resolved Hide resolved

venkyyuvy added 2 commits October 27, 2019 19:40

format fix

fb5ff71

getattr option

db8e23e

jnothman reviewed Oct 27, 2019

View reviewed changes

sklearn/feature_selection/rfe.py Outdated Show resolved Hide resolved

sklearn/feature_selection/rfe.py Outdated Show resolved Hide resolved

sklearn/feature_selection/rfe.py Outdated Show resolved Hide resolved

venkyyuvy added 2 commits October 28, 2019 11:04

including string support

1ace4f9

including string option

4210392

venkyyuvy added 4 commits October 28, 2019 14:16

format fix

47fa867

forwarding

49ef205

Merge branch 'param_importance' of https://github.com/venkyyuvy/sciki…

f69aa8e

…t-learn into param_importance

adding missing import

db55c84

jnothman reviewed Oct 30, 2019

View reviewed changes

sklearn/feature_selection/_rfe.py Show resolved Hide resolved

sklearn/feature_selection/_rfe.py Outdated Show resolved Hide resolved

venkyyuvy added 3 commits November 3, 2019 23:28

add examples and testing

633beb2

Merge remote-tracking branch 'upstream/master' into param_importance

92685a8

format fix

58818e0

jnothman reviewed Nov 4, 2019

View reviewed changes

sklearn/feature_selection/_rfe.py Outdated Show resolved Hide resolved

sklearn/feature_selection/_rfe.py Outdated Show resolved Hide resolved

glemaitre reviewed May 13, 2020

View reviewed changes

doc/whats_new/v0.23.rst Outdated Show resolved Hide resolved

glemaitre reviewed May 13, 2020

View reviewed changes

sklearn/feature_selection/_rfe.py Show resolved Hide resolved

glemaitre reviewed May 13, 2020

View reviewed changes

sklearn/feature_selection/_rfe.py Outdated Show resolved Hide resolved

sklearn/feature_selection/_rfe.py Show resolved Hide resolved

glemaitre reviewed May 13, 2020

View reviewed changes

glemaitre moved this from TO REVIEW to REVIEWED AND WAITING FOR CHANGES in Guillaume's pet May 13, 2020

venkyyuvy added 6 commits May 13, 2020 19:32

adding hyperlink to classes

cb52a9d

changing svr to LinearSVR

488db87

docstring update

bb7191a

updating with linearSVR

3febbb8

linting fix

40e1c21

more linting fix

b996ea3

glemaitre reviewed May 13, 2020

View reviewed changes

sklearn/feature_selection/tests/test_rfe.py Outdated Show resolved Hide resolved

venkyyuvy added 3 commits May 14, 2020 16:22

update exp. features for rfecv

388b293

removing attrgetter in docstring

renaming ranking_func to transform_func

dd0210a

moving change log to 0.24

6ebc1f3

glemaitre reviewed May 14, 2020

View reviewed changes

adding docstring to get_fea_imp

6bb0cce

renaming expected_features to expected_n_features

glemaitre approved these changes May 19, 2020

View reviewed changes

sklearn/feature_selection/_from_model.py Outdated Show resolved Hide resolved

sklearn/feature_selection/_from_model.py Outdated Show resolved Hide resolved

venkyyuvy added 2 commits May 20, 2020 09:06

versionadded tag and pos. arg change

701ac31

user guide update

d89edc3

jnothman approved these changes May 20, 2020

View reviewed changes

sklearn/feature_selection/_from_model.py Show resolved Hide resolved

jnothman changed the title ~~ENH add importance_getter to RFE* to get importances from meta-estimators~~ ENH add importance_getter to RFE* and SelectFromModel May 20, 2020

jnothman merged commit f6b8bc0 into scikit-learn:master May 20, 2020

glemaitre moved this from REVIEWED AND WAITING FOR CHANGES to MERGED in Guillaume's pet May 20, 2020

thomasjpfan mentioned this pull request Apr 23, 2022

Retrieve feature importances for any model or any that SelectFromModel supports #9110

Closed

ENH add importance_getter to RFE* and SelectFromModel #15361

ENH add importance_getter to RFE* and SelectFromModel #15361

venkyyuvy commented Oct 25, 2019

jnothman commented Oct 26, 2019 via email

venkyyuvy commented Oct 27, 2019

jnothman left a comment

venkyyuvy commented Oct 28, 2019

glemaitre commented May 13, 2020

glemaitre left a comment

glemaitre May 13, 2020

glemaitre May 13, 2020

glemaitre left a comment

venkyyuvy commented May 15, 2020

glemaitre commented May 19, 2020

glemaitre left a comment

venkyyuvy commented May 20, 2020

jnothman commented May 20, 2020

ENH add importance_getter to RFE* and SelectFromModel #15361

ENH add importance_getter to RFE* and SelectFromModel #15361

Conversation

venkyyuvy commented Oct 25, 2019

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Any other comments?

jnothman commented Oct 26, 2019 via email

venkyyuvy commented Oct 27, 2019

jnothman left a comment

Choose a reason for hiding this comment

venkyyuvy commented Oct 28, 2019

glemaitre commented May 13, 2020

glemaitre left a comment

Choose a reason for hiding this comment

glemaitre May 13, 2020

Choose a reason for hiding this comment

glemaitre May 13, 2020

Choose a reason for hiding this comment

glemaitre left a comment

Choose a reason for hiding this comment

venkyyuvy commented May 15, 2020

glemaitre commented May 19, 2020

glemaitre left a comment

Choose a reason for hiding this comment

venkyyuvy commented May 20, 2020

jnothman commented May 20, 2020