Skip to content

DRAFT: Implements fit params for RFECV #21113

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

Closed
wants to merge 1 commit into from

Conversation

fbidu
Copy link
Contributor

@fbidu fbidu commented Sep 23, 2021

Reference Issues/PRs

Fixes #7308

What does this implement/fix? Explain your changes.

Fit params for RFECV

Any other comments?

This is a draft PR and it is broken. I uploaded just so I can keep my work more visible

@adrinjalali
Copy link
Member

This opens up a few issues, such as not passing those **fit_params to the scorer or the splitter. #20350 adds the support with the new proposed API

@fbidu
Copy link
Contributor Author

fbidu commented Sep 23, 2021

@adrinjalali oh that's a bummer. My intent here was to "Just" copy the feature I implemented on #20380 to RFECV. Do you have any suggestions on going forward?

@adrinjalali
Copy link
Member

One reason this wasn't implemented in that other PR is exactly these challenges. We're really hoping to have the props PR merged for the next major release. That would be the ideal path forward.

@fbidu
Copy link
Contributor Author

fbidu commented Oct 10, 2021

@adrinjalali thanks for clearing that up. I'll wait before that is implemented to tackle this. Do you think it is better to close this PR?

@adrinjalali
Copy link
Member

yeah I think so.

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

Successfully merging this pull request may close these issues.

RFE/RFECV doesn't work with sample weights
2 participants