Fixed #2195, improved ConfusionMatrix docs #2196
Conversation
There was a problem hiding this comment.
Thanks for the PR @mfoglio
Looks good, I was just thinking about moving this to the end of the docstring as a note.
Btw, we can see rendered docstring here: https://deploy-preview-2196--pytorch-ignite-preview.netlify.app/generated/ignite.metrics.confusion_matrix.confusionmatrix#ignite.metrics.confusion_matrix.ConfusionMatrix
ignite/metrics/confusion_matrix.py
Outdated
| @@ -13,6 +13,10 @@ | |||
| class ConfusionMatrix(Metric): | |||
| """Calculates confusion matrix for multi-class data. | |||
|
|
|||
| The confusion matrix is formatted such that columns are predictions and rows are targets. | |||
There was a problem hiding this comment.
Let's move this details below after the line L67 and use Note: :
Note:
The confusion matrix is formatted such that columns are predictions and rows are targets.
For example, if you were to plot the matrix, you could correctly assign to the horizontal axis
the label "predicted values" and to the vertical axis the label "actual values".There was a problem hiding this comment.
Sounds great! @vfdev-5 , should I do the edit? If yes, how? :)
There was a problem hiding this comment.
Yes, let's edit the file locally as suggested. Next, commit and push: git commit -a -m "Updated docstring" && git push origin fix-docs-2195
There was a problem hiding this comment.
Done. Let me know if that's fine.
There was a problem hiding this comment.
Yes, IMO looks good now! Thanks for the PR !
There was a problem hiding this comment.
Thank you for the guidance!
Fixes #2195
Description: add information about the format of confusion matrix in the docs
Check list: