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

[WIP] Rewrite stacking estimators with class hierarchy #445

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

kota7
Copy link
Contributor

@kota7 kota7 commented Sep 25, 2018

Description

This is work-in-progress

Refactors existing stacking estimators with class inheritance structure. More specifically, classes are related as below. The real code is a bit more complex with multiple inheritance.

StackingEstimator (new, "abstract")
   |-- StackingRegressor
   |-- StackingClassifier
   |-- StackingCVEstimator (new, "abstract")
       |-- StackingCVRegressor
       |-- StackingCVClassifier

In addition, following arguments are renamed:

  • refit --> use_clones in StackingRegressor and StackingCVRegressor
    Following option is added
  • verbose to StackingRegressor

Although there are still some tests fail and bug fix is going, I would like to have your thoughts as of now.

Fixes #444

Related issues or pull requests

Pull Request Checklist

  • Added a note about the modification or contribution to the ./docs/sources/CHANGELOG.md file (if applicable)
  • Added appropriate unit test functions in the ./mlxtend/*/tests directories (if applicable)
  • Modify documentation in the corresponding Jupyter Notebook under mlxtend/docs/sources/ (if applicable)
  • Ran nosetests ./mlxtend -sv and make sure that all unit tests pass (for small modifications, it might be sufficient to only run the specific test file, e.g., nosetests ./mlxtend/classifier/tests/test_stacking_cv_classifier.py -sv)
  • Checked for style issues by running flake8 ./mlxtend

@pep8speaks
Copy link

Hello @kota7! Thanks for submitting the PR.

Line 281:4: W292 no newline at end of file

Line 12:44: W292 no newline at end of file

Line 16:43: W292 no newline at end of file

Line 72:80: E501 line too long (80 > 79 characters)

Line 136:1: E302 expected 2 blank lines, found 1

Line 249:40: W291 trailing whitespace
Line 394:32: W292 no newline at end of file

Line 11:58: W292 no newline at end of file

Line 26:50: W291 trailing whitespace
Line 34:1: W293 blank line contains whitespace
Line 36:9: E265 block comment should start with '# '
Line 58:12: W291 trailing whitespace
Line 62:1: W293 blank line contains whitespace
Line 66:1: W293 blank line contains whitespace
Line 67:9: E303 too many blank lines (2)
Line 71:1: W293 blank line contains whitespace
Line 75:56: W291 trailing whitespace
Line 77:1: W293 blank line contains whitespace
Line 100:1: W293 blank line contains whitespace
Line 151:1: W293 blank line contains whitespace
Line 157:1: W293 blank line contains whitespace
Line 176:1: W293 blank line contains whitespace
Line 196:14: W291 trailing whitespace
Line 198:28: E502 the backslash is redundant between brackets
Line 200:1: W293 blank line contains whitespace
Line 203:40: E502 the backslash is redundant between brackets
Line 205:1: W293 blank line contains whitespace
Line 206:1: E302 expected 2 blank lines, found 1
Line 253:80: E501 line too long (80 > 79 characters)
Line 264:1: W293 blank line contains whitespace
Line 283:1: W293 blank line contains whitespace
Line 295:1: W293 blank line contains whitespace
Line 303:1: W293 blank line contains whitespace
Line 371:1: W293 blank line contains whitespace
Line 383:45: W291 trailing whitespace
Line 439:1: W293 blank line contains whitespace
Line 446:30: W292 no newline at end of file

Line 67:1: W293 blank line contains whitespace
Line 73:1: W293 blank line contains whitespace
Line 81:56: W291 trailing whitespace
Line 85:1: W293 blank line contains whitespace
Line 86:38: W291 trailing whitespace
Line 92:1: W293 blank line contains whitespace
Line 121:65: E502 the backslash is redundant between brackets
Line 122:40: E131 continuation line unaligned for hanging indent
Line 132:68: W291 trailing whitespace
Line 146:1: W293 blank line contains whitespace
Line 239:1: W293 blank line contains whitespace
Line 352:1: W293 blank line contains whitespace
Line 382:1: W293 blank line contains whitespace

@kota7 kota7 changed the title [WIP] Define stacking estimators with class hierarchy [WIP] Rewrite stacking estimators with class hierarchy Sep 25, 2018
@rasbt
Copy link
Owner

rasbt commented Sep 28, 2018

Thanks for the PR!

Just noticed a lot of failing unit tests. I think this may be because sklearn 0.20 has recently become available via miniconda, and mlxtend (also) tests the lastest sklearn version. There were some bugfixes to some estimators plus a slight change in the iris dataset (two data points changed) which explained some of the different results. Adjusted the unit tests yesterday and will rerun the CI tests here, then we can see if there are some issues remaining.

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.

Refactor stacking estimators with class inheritance
3 participants