Skip to content

Conversation

ClimbsRocks
Copy link
Contributor

Fixes #6564.

Does not change any of the logic, simply allows the user to optionally pass in a value for n_features_to_select, rather than hard-coding in the value of 1. Still defaults to 1 if the user does not pass in a value.

@jnothman
Copy link
Member

Needs a test

@@ -313,6 +313,11 @@ class RFECV(RFE, MetaEstimatorMixin):
Defaults to 1 core. If `n_jobs=-1`, then number of jobs is set
to number of cores.

n_features_to_select : int, default=1
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be min_n_features, no?

Copy link
Member

Choose a reason for hiding this comment

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

why?

Copy link
Member

Choose a reason for hiding this comment

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

Does this param dictate exactly how many features are selected, or a minimum?

Copy link

Choose a reason for hiding this comment

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

I believe this does not precisely address #6564 since setting a fixed number of final features (n_features_to_select) is different from setting a minimum (min_n_features). I have posted #8812 in an attempt to address this difference.

@amueller
Copy link
Member

amueller commented Aug 5, 2019

fixed in #11293

@amueller amueller closed this Aug 5, 2019
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.

Class RFECV needs hard-coded n_features_to_select as a input parameter.
4 participants