[MRG + 1] Deprecate estimator_params in RFE and RFECV #4292 #4368
Conversation
| estimator_params={}, verbose=0): | ||
| estimator_params=None, verbose=0): | ||
| if estimator_params is not None: | ||
| warnings.warn("The 'estimator parameters' is deprecated as of version 0.16 " |
There was a problem hiding this comment.
You should use the actual name estimator_params and either say Just "estimator_params is deprecated" or "The parameterestimator_params``" to make a complete english sentence.
There was a problem hiding this comment.
Also, this should be done in fit, not in __init__.
|
Flagging for 0.16 so we can backport and can remove earlier. |
|
Travis failure seems unrelated, I restarted. |
|
Thanks! Do you mean the warning message should be 'The parameter estimator_params will be removed in 0.16.'? |
|
The "flagging for 0.16" was a note to myself, @ogrisel or whoever else want's to release ;) The message should say
|
|
@amueller Fixed. |
| rfe = RFE(estimator=SVC(), n_features_to_select=4, step=0.1, | ||
| estimator_params={'kernel': 'linear'}) | ||
| assert_warns_message(DeprecationWarning, deprecation_message, | ||
| rfe.fit, |
There was a problem hiding this comment.
You can put this on the same line as the two below.
|
LGTM apart from style comments above. |
|
LGTM as well. Merging. |
[MRG + 1] Deprecate estimator_params in RFE and RFECV #4292
|
The parameter should have been removed from the docstring, sorry, I overlooked that. @xuewei4d do you want to do that? |
|
No problem. I will open a new PR. @amueller |
|
Thanks :) |
Fix #4292. Is the warning message OK?