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

[MRG] DOC make confusion_matrix_plot.py example more copy-pastable #6966

Merged
merged 1 commit into from
Jul 15, 2016

Conversation

doshyt
Copy link
Contributor

@doshyt doshyt commented Jul 7, 2016

Reference Issue

Addressed: #6796

What does this implement/fix? Explain your changes.

  • Make plot_confusion_matrix.py example more copy-pastable by putting the calculated numbers into the middle of each square on the plot
  • Use the list of class names as a parameter of the plotting function

@jnothman
Copy link
Member

jnothman commented Jul 8, 2016

Can you save us some time by providing an image?

@doshyt
Copy link
Contributor Author

doshyt commented Jul 8, 2016

@jnothman do you mean plots for confusion matrices that the code generates? I was looking for the proper location of these images to put them to the repo but didn't find it. I attach them here, please point me to the correct location if I'm wrong.

confusion_matrix_plot_001
confusion_matrix_plot_002

@nelson-liu
Copy link
Contributor

I think it would be nice to have a parameter normalize that automatically does the normalizing for the user if set to true, if the intention is to make the example more copy-pastable

also seems like the appveyor failure is spurious on both agramfort's account and the sklearn-ci account. see: https://ci.appveyor.com/project/sklearn-ci/scikit-learn/build/1.0.6980?

@doshyt
Copy link
Contributor Author

doshyt commented Jul 14, 2016

Thanks, @nelson-liu for valuable comments. Indeed, moving the normalization into the function body seems like a good improvement.
I updated the example and also made a few other cosmetic changes to make it shiny and easy to read.

@nelson-liu
Copy link
Contributor

this looks like a good change to me; in the future, please avoid squashing commits during development.

@amueller
Copy link
Member

lgtm.

@amueller amueller merged commit 97c47d9 into scikit-learn:master Jul 15, 2016
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