-
Notifications
You must be signed in to change notification settings - Fork 237
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
Adding metric visualization #342
Conversation
The documentation is not available anymore as the PR was closed or merged. |
@lvwerra lmk what you think now. Also, where is the best place to document the viz function? in the evaluate docs directly, or via a README? |
If we plan on building out more visualizations, would it be worth committing to writing this module with the OO-interface for matplotlib? As far as I know it would make testing easier, and I think it would be more dev-friendly for composability + subclassing to extend functionality if folks have custom visualizations that they want to build out. |
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks really good @sashavor, congrats on mastering unittests 🎉 Left a comments and then I think it's good to go!
One thing that's currently missing is documentation. I think it would be great to have some docs on this. E.g. create a new section |
Also, I don't understand the CircleCI errors 😬 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only minor things left to do. Can you use the following format for the docstrings:
https://github.com/huggingface/doc-builder#writing-source-documentation
Co-authored-by: Leandro von Werra <lvwerra@users.noreply.github.com>
Co-authored-by: Leandro von Werra <lvwerra@users.noreply.github.com>
Co-authored-by: Leandro von Werra <lvwerra@users.noreply.github.com>
Co-authored-by: Leandro von Werra <lvwerra@users.noreply.github.com>
The CI is green now :) I think there are just two more things left to do:
|
I've currently added the code from ComplexRadar() and it does work, but I can't figure out at what point it makes most sense to add the
invert_range
flag@lvwerra would love your help!