search

hyperopt / hyperopt-sklearn

Hyper-parameter optimization for sklearn
hyperopt.github.io/hyperopt-sklearn
Other
1.58k stars 272 forks source link

Adding GroupKFold and testing it #200

Closed hyun-seo closed 11 months ago

hyun-seo commented 1 year ago

Referring to #157, there is an issue about group cross-validation.

This pull request implementation this issue by adding new argument kfolds_group and using sklearn.model_selection.GroupKFold.

hyun-seo commented 1 year ago

@mandjevant Thank you so much for taking the time to review my pr. This is my first attempt at contributing to open source and I had a lot of issues, but I think I learned a lot from your kind help. I have one question: what is the exact meaning of # noqa: E226 in the testcode? Why is this phrase needed in the test?

mandjevant commented 1 year ago

@mandjevant Thank you so much for taking the time to review my pr. This is my first attempt at contributing to open source and I had a lot of issues, but I think I learned a lot from your kind help. I have one question: what is the exact meaning of # noqa: E226 in the testcode? Why is this phrase needed in the test?

Thank you for your contribution! The reason your PR did not go through automated testing was because of flake8 style guide enforcement. In most lines, flake was correct and notified us of style issues that we resolved. But in that one line, I deemed that flake8 was wrong. It threw an inconsistency with rule E226 which, when enforced, would make the line unreadable.

Thus, I suppressed the rule on (only) that line by adding a comment stating noqa: E226.

mandjevant commented 11 months ago

Ill go ahead and merge this. Thank you for your contribution @hyun-seo