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

MRG Rfe estimator params #1128

Merged
merged 0 commits into from Sep 12, 2012

Conversation

Projects
None yet
3 participants
@amueller
Member

amueller commented Sep 7, 2012

This addresses issue #1028 and part of #1115:
RFE no clones the estimator before changing it, therefore being more strict in the "fit doesn't change parameters" sense. Also other meta-estimators like GridSearch behave this way.
This is unfortunately a slight API breakage as now, after fitting rfe.estimator is an unfitted estimator and rfe.estimator_ is a fitted one.
The rfe.estimator was not documented, though.

The other commit makes it possible to use RFE and RFECV in a grid search. As for example linear classifiers basically always need a grid search for C I think this is an important feature.

@ogrisel

This comment has been minimized.

Show comment
Hide comment
@ogrisel

ogrisel Sep 7, 2012

Member

LGTM.

Member

ogrisel commented Sep 7, 2012

LGTM.

@amueller

This comment has been minimized.

Show comment
Hide comment
@amueller

amueller Sep 7, 2012

Member

I fixed a problem in sparse RFECV that I overlooked before and another bug in RFECV.
Also added a "verbose" parameter as this takes forever (in particular with steps=1 as was always the case before).

Member

amueller commented Sep 7, 2012

I fixed a problem in sparse RFECV that I overlooked before and another bug in RFECV.
Also added a "verbose" parameter as this takes forever (in particular with steps=1 as was always the case before).

@agramfort

This comment has been minimized.

Show comment
Hide comment
@agramfort

agramfort Sep 9, 2012

Member

LGTM

Member

agramfort commented Sep 9, 2012

LGTM

@amueller amueller merged commit ea3f32b into scikit-learn:master Sep 12, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment