Skip to content
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

FEA Add RepeatedStratifiedGroupKFold as a new splitter #24227

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

arvkevi
Copy link

@arvkevi arvkevi commented Aug 23, 2022

closes #24247

Reference Issues/PRs

This functionality was discussed in #13621.

What does this implement/fix? Explain your changes.

This adds a splitter class to model_selection that repeats StratifiedGroupKFold n times.

Any other comments?

@arvkevi arvkevi changed the title Add RepeatedStratifiedGroupKFold [MRG] Add RepeatedStratifiedGroupKFold Oct 2, 2023
@glemaitre glemaitre changed the title [MRG] Add RepeatedStratifiedGroupKFold FEA Add RepeatedStratifiedGroupKFold as a new splitter Mar 13, 2024
@glemaitre glemaitre self-requested a review March 13, 2024 10:33
Copy link

github-actions bot commented Mar 13, 2024

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: 63ae5be. Link to the linter CI: here

@glemaitre
Copy link
Member

I'll provide a review to this feature. Sorry @arvkevi that this PR did not get any attention.

@arvkevi are you still willing to address the review that I'll be doing or you prefer to give the torch to someone else?

@arvkevi
Copy link
Author

arvkevi commented Mar 13, 2024

No worries, I can address the review @glemaitre

@adrinjalali
Copy link
Member

I think for this it makes sense to have a repeat kwarg to the constructor and have a variant instead of a new class. Note that with the existing classes we might also merge them together and deprecated some.

@glemaitre glemaitre added this to the 1.6 milestone May 20, 2024
@glemaitre
Copy link
Member

@adrinjalali I think that we can add the class and the deprecation can come as a whole where we move into parameters instead of separate class.

@glemaitre
Copy link
Member

In general, it looks good. Just a couple of nitpicks.

@glemaitre glemaitre removed their request for review May 20, 2024 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

Add RepeatedStratifiedGroupKFold
3 participants