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 class names param to plot_confusion_matrix #487

Merged
merged 3 commits into from
Mar 27, 2019
Merged

Conversation

pokidyshev
Copy link
Contributor

@pokidyshev pokidyshev commented Jan 1, 2019

Description

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

pep8speaks commented Jan 1, 2019

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

Line 100:1: W293 blank line contains whitespace
Line 105:1: W293 blank line contains whitespace

Comment last updated at 2019-03-27 19:09:58 UTC

@coveralls
Copy link

coveralls commented Jan 1, 2019

Coverage Status

Coverage decreased (-0.07%) to 91.543% when pulling 170ffa2 on sandpiturtle:master into 5016a00 on rasbt:master.

@rasbt
Copy link
Owner

rasbt commented Jan 2, 2019

Thanks for the PR. This is a nice addition according to the discussion in #485

Since there are no unit tests for plotting visualizations, could you please add an example in the documentation (at the bottom of https://github.com/rasbt/mlxtend/blob/master/docs/sources/user_guide/plotting/plot_confusion_matrix.ipynb ; these jupyter notebooks are used to render the HTML documentation upon a new version release)

Also, it would be nice if you could make an entry in the Changelog.

@rasbt rasbt merged commit a4be7c5 into rasbt:master Mar 27, 2019
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