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

Adds fit_params support for ExhaustiveFeatureSelector #354

Merged
merged 6 commits into from
Mar 27, 2018

Conversation

zdgriffith
Copy link
Contributor

@zdgriffith zdgriffith commented Mar 26, 2018

Description

Adds support for fit parameters to be passed to the fit method of ExhaustiveFeatureSelector.

Related issues or pull requests

Related to Pull Requests #255 , #350

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 Mar 26, 2018

Hello @zdgriffith! Thanks for updating the PR.

Line 405:80: E501 line too long (84 > 79 characters)
Line 417:80: E501 line too long (81 > 79 characters)

Comment last updated on March 27, 2018 at 22:00 Hours UTC

@coveralls
Copy link

coveralls commented Mar 26, 2018

Coverage Status

Coverage increased (+0.02%) to 91.746% when pulling d1ef89f on zdgriffith:efs-fit-params into c019c87 on rasbt:master.

@zdgriffith
Copy link
Contributor Author

Sorry, I probably should have fit this into #350 .

@rasbt
Copy link
Owner

rasbt commented Mar 26, 2018

Thanks, Zach! And no worries, I think this way (having two PRs) makes it less confusing with regard to checking what's changed. The PR looks good to me, could you maybe also update the ExhaustiveFeatuerSelector.ipynb ? I.e., the last cell with the API documentation. You simply need to cd into mlxtend/docs, run python make_api.py and then run the last cell in the notebook

@zdgriffith
Copy link
Contributor Author

Sorry about that, the notebook is updated now!

@rasbt
Copy link
Owner

rasbt commented Mar 27, 2018

Thanks a lot!

@rasbt rasbt merged commit 9c8529a into rasbt:master Mar 27, 2018
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.

None yet

4 participants