[MRG] Add fit_params to RFECV.fit #24004
Conversation
There was a problem hiding this comment.
@ScottMGustafson Thank you for the PR! I'm thinking if we should accept this PR as is because there has been progress on meta-data routing work. The meta-data routing framework would allow RFECV to route any metadata to the inner estimator, splitter, or scorer. See SLEP006 for details.
@adrinjalali Do you think we should accept his PR in it's current state given that we are planning to migrate to the meta-data routing framework soon?
|
Since we're hoping to have metadata routing in the coming release, I'd say rather not merge this one. It makes noticing when to go with deprecation and when to just introduce routing easier w/o it. @ScottMGustafson happy to have a PR for RFECV in the context of this issue though: #22893, but it's quite a bit more involved than this PR. |
|
With #24004 (comment), I am closing this PR since we are moving to the meta-data routing API. @ScottMGustafson As noted in the same comment, you are welcome to open another PR for RFECV in context of #22893. |
Reference Issues/PRs
Fixes #17954
What does this implement/fix? Explain your changes.
Add support for
**fit_paramstoRFECV.fitand added a test to check that these parameters are passed to the underlying estimator.Any other comments?