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

Add stacking param tuning #254

Merged
merged 5 commits into from
Sep 20, 2017
Merged

Conversation

jrbourbeau
Copy link
Contributor

@jrbourbeau jrbourbeau commented Sep 20, 2017

Description

Currently the parameters in StackingClassifier, StackingCVClassifier, StackingRegressor, StackingCVRegressor, and EnsembleVoteClassifier can't be tuned in GridSearchCV. This PR fixes that bug.

Related issues or pull requests

Fixes #253

Pull Request requirements

  • Added appropriate unit test functions in the ./mlxtend/*/tests directories
  • Ran nosetests ./mlxtend -sv and make sure that all unit tests pass
  • Checked the test coverage by running nosetests ./mlxtend --with-coverage
  • Checked for style issues by running flake8 ./mlxtend
  • Added a note about the modification or contribution to the ./docs/sources/CHANGELOG.md file
  • Modify documentation in the appropriate location under mlxtend/docs/sources/ (optional)
  • Checked that the Travis-CI build passed at https://travis-ci.org/rasbt/mlxtend

@pep8speaks
Copy link

pep8speaks commented Sep 20, 2017

Hello @jrbourbeau! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on September 20, 2017 at 03:24 Hours UTC

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 89.649% when pulling 67df542 on jrbourbeau:add_stacking_param_tuning into edb96f1 on rasbt:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 89.657% when pulling 16c7772 on jrbourbeau:add_stacking_param_tuning into edb96f1 on rasbt:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 89.657% when pulling 16c7772 on jrbourbeau:add_stacking_param_tuning into edb96f1 on rasbt:master.

@jrbourbeau
Copy link
Contributor Author

@rasbt I think this PR might be good to go. Let me know if you see any issues!

Note: I didn't make any additions to the tests for StackingRegressor because there aren't any additional parameters other than verbose! But if there more parameters are added in the future, they should be able to work in GridSearchCV.

@rasbt
Copy link
Owner

rasbt commented Sep 20, 2017

Thanks a lot, I really appreciate that! The PR looks perfect, and I think that although StackingRegressor doesn't have any additional parameters, it's good to made the mod already (even though there's no unit test for that yes).

@rasbt rasbt merged commit 3424df6 into rasbt:master Sep 20, 2017
@jrbourbeau jrbourbeau deleted the add_stacking_param_tuning branch September 22, 2017 02:24
@jrbourbeau
Copy link
Contributor Author

No problem, thanks for all your work on mlxtend!

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

Successfully merging this pull request may close these issues.

Also allow tuning of the Stacking parameters
4 participants