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

EnsembleVotingRegressor #602 #648

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

Conversation

zzcysta
Copy link

@zzcysta zzcysta commented Dec 20, 2019

Description

-Added EnsembleVotingRegressor in regressor folder
-also including my own test file in the test folder.

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 PYTHONPATH='.' pytest ./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., PYTHONPATH='.' pytest ./mlxtend/classifier/tests/test_stacking_cv_classifier.py -sv)
  • Checked for style issues by running flake8 ./mlxtend

-Added EnsembleVotingRegressor in regressor folder
-also including my own test file in the test folder.
@pep8speaks
Copy link

pep8speaks commented Dec 20, 2019

Hello @zzcysta! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 13:80: E501 line too long (80 > 79 characters)
Line 21:80: E501 line too long (82 > 79 characters)
Line 59:80: E501 line too long (85 > 79 characters)
Line 121:5: E303 too many blank lines (3)
Line 138:27: E127 continuation line over-indented for visual indent

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

Line 13:80: E501 line too long (80 > 79 characters)
Line 21:80: E501 line too long (82 > 79 characters)
Line 59:80: E501 line too long (85 > 79 characters)
Line 121:5: E303 too many blank lines (3)
Line 138:27: E127 continuation line over-indented for visual indent

Line 133:35: E127 continuation line over-indented for visual indent
Line 134:35: E127 continuation line over-indented for visual indent
Line 278:35: E127 continuation line over-indented for visual indent
Line 279:35: E127 continuation line over-indented for visual indent
Line 285:35: E127 continuation line over-indented for visual indent
Line 286:35: E127 continuation line over-indented for visual indent
Line 298:35: E127 continuation line over-indented for visual indent
Line 299:35: E127 continuation line over-indented for visual indent

Comment last updated at 2019-12-21 21:51:03 UTC

@qiagu
Copy link
Contributor

qiagu commented Dec 20, 2019

I didn't look closely into the code, but it sounds somewhat similar to sklearn.ensemble.VotingRegressor. Please feel free to correct me if I were wrong.

@zzcysta
Copy link
Author

zzcysta commented Dec 21, 2019

I didn't look closely into the code, but it sounds somewhat similar to sklearn.ensemble.VotingRegressor. Please feel free to correct me if I were wrong.

Yes, I did take a look at what EnsembleAVoting_Regressor was like in scikit-learn, I take it as a guidance and I also see how ensemble voting classifier work in mlxtend. Overall there are some differences, like the form of passing parameters is different. But both of them can get the same result.

@qiagu
Copy link
Contributor

qiagu commented Dec 21, 2019

Interesting! Happy to know that.

@rasbt
Copy link
Owner

rasbt commented Dec 21, 2019

A bit of a background story, the VotingClassifier in scikit-learn actually comes from the one in mlxtend. I had a very rudimentary version in my Python Machine Learning book in 2015 and added a more comprehensive version to mlxtend. The community liked it which is why we decided to make a version for scikit-learn. When I worked in the PR of the VotingClassifier in mlxtend, the scikit-learn team suggest a few parameter renaminings and changes, which is why they now look a bit different. Over time I also added a few modifications to the VotingClassifier -- by user request -- that are currently not in scikit-learn.

In any case, I didn't know that there was a VotingRegressor added in scikit-learn in the mean time -- the issue here was relatively old. However, it doesn't hurt to have a EnsembleVotingRegressor here that is consistent with the EnsembleVotingClassifier UI-wise.

@rasbt
Copy link
Owner

rasbt commented Dec 21, 2019

Regarding this PR, I just see that there is a file

mlxtend/regressor/tests/test_ensemble_voting_regressor.ipynb 

However, the test file should be a regular .py script similar to https://github.com/rasbt/mlxtend/blob/master/mlxtend/classifier/tests/test_ensemble_vote_classifier.py

The jupyter documentation ensemble_voting_regressor.ipynb should be in docs/sources/user_guide/regressor

@zzcysta
Copy link
Author

zzcysta commented Dec 21, 2019

Sorry for putting the file in the wrong folder I will change it and make another pull request. For the test file, I forget to take a look at the form of test file so I made my own one to see whether it can out put a correct result or not. I will make the change asap. Thanks for reminding me that.

@rasbt
Copy link
Owner

rasbt commented Dec 21, 2019

No worries! Please do not make a new PR though. You can just update this existing PR. After making the changes, simply git add and git commit these changes and then just use git push origin Ensemble_voting_Regressor -- this will update this PR automatically.

@qiagu
Copy link
Contributor

qiagu commented Dec 21, 2019

A bit of a background story, the VotingClassifier in scikit-learn actually comes from the one in mlxtend. I had a very rudimentary version in my Python Machine Learning book in 2015 and added a more comprehensive version to mlxtend. The community liked it which is why we decided to make a version for scikit-learn. When I worked in the PR of the VotingClassifier in mlxtend, the scikit-learn team suggest a few parameter renaminings and changes, which is why they now look a bit different. Over time I also added a few modifications to the VotingClassifier -- by user request -- that are currently not in scikit-learn.

In any case, I didn't know that there was a VotingRegressor added in scikit-learn in the mean time -- the issue here was relatively old. However, it doesn't hurt to have a EnsembleVotingRegressor here that is consistent with the EnsembleVotingClassifier UI-wise.

Cool story and admire the Python Machine Learning book. The scikit-learn VotintRegressor was added as new in v0.21, so not long ago.

Hi professor,
I have move ensemble_vote_regressor into the right folder and also change the form of test file. However, I can't fully understand the working flow of the test file so I just imitate the test file for ensemble_voting_classifer.
I have also include my own test outputs which is using graph to compare the performance of EnsembleVoting, Stacking and Voting in Regressor.

It's my first time trying to tackle such problem. So I am willing to receive any kinds of advices and please feel free to correct any of my mistakes!
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.

4 participants