8000 MAINT Use check_scalar in _BaseStacking by genvalen · Pull Request #22405 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

MAINT Use check_scalar in _BaseStacking #22405

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 11 commits into from
Apr 17, 2022
Merged

MAINT Use check_scalar in _BaseStacking #22405

merged 11 commits into from
Apr 17, 2022

Conversation

genvalen
Copy link
Contributor
@genvalen genvalen commented Feb 7, 2022 •
edited
Loading

Reference Issues/PRs

Addresses #20724 and #21927
#DataUmbrella

What does this implement/fix? Explain your changes.

Summary of changes to BaseGradientBoosting:

  • Identify scalar parameters.
  • Add tests to ensure
    StackingClassifier and
    StackingRegressor
    raise proper errors when invalid arguments are passed in.
  • Use the helper function check_scalar from sklearn.utils to validate the scalar parameters.

Test and validation progress:

  • cv (cv & other splitter classes will be validated in a separate PR )
  • passthrough
  • verbose (per previous convos, this doesn't need to be validated for now -- I'll focus on hyperparams)

Any other comments?

Please advise correct type/range if different than what I have below. (Thank you):
param: type-> range
cv: int-> (2, inf]
passthrough: np.bool_, or int -> [-inf, inf]
verbose: int-> np.bool_, or [0, inf)

@genvalen genvalen marked this pull request as draft February 23, 2022 21:06
@genvalen genvalen changed the title [WIP] MAINT Use check_scalar in _BaseStacking [MRG] MAINT Use check_scalar in _BaseStacking Mar 18, 2022
@genvalen genvalen marked this pull request as ready for review March 18, 2022 02:45
@genvalen
Copy link
Contributor Author

Hi @glemaitre, please review this when you are able.

Also FYI, based on a conversation with @thomasjpfan a few community sessions ago, I'm going to work on the splitter classes in a separate PR.

@genvalen @glemaitre
Update sklearn/ensemble/_stacking.py
2e7be5f 
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
ogrisel
ogrisel approved these changes Mar 21, 2022
View reviewed changes
Copy link
Member
@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM if CI is green.

@thomasjpfan thomasjpfan changed the title [MRG] MAINT Use check_scalar in _BaseStacking MAINT Use check_scalar in _BaseStacking Apr 17, 2022
@thomasjpfan thomasjpfan merged commit 65ae1a8 into scikit-learn:main Apr 17, 2022
jjerphan pushed a commit to jjerphan/scikit-learn that referenced this pull request Apr 29, 2022
@genvalen @glemaitre
@ogrisel @jjerphan
MAINT Use check_scalar in _BaseStacking (scikit-learn#22405)
53184e9 
Co-authored-by: Guillaume Lemaitre <g.lemaitre58@gmail.com>
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
@genvalen genvalen deleted the stacking_add_check_scalar branch March 10, 2023 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@glemaitre glemaitre glemaitre left review comments

@ogrisel ogrisel ogrisel approved these changes

@thomasjpfan thomasjpfan thomasjpfan approved these changes

Assignees
No one assigned
Labels
module:ensemble No Changelog Needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@genvalen @ogrisel @thomasjpfan @glemaitre
0