-
-
Notifications
You must be signed in to change notification settings - Fork 654
Added sub heading to ignite.metrics #2448
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
Conversation
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.
Thanks for the draft PR @sayantan1410
Let's improve text
@vfdev-5 Hey, made the changes please check !! |
@vfdev-5 Hey I have made some changes, let me know if they are alright !! |
@sayantan1410 can you reread the docs https://deploy-preview-2448--pytorch-ignite-preview.netlify.app/metrics.html#ignite-metrics and critically estimate how more comprehensive or cumbersome docs are becoming now ? |
@vfdev-5 Hey, I tried to make it simpler and consistent, can you please check it once more. |
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 much better now, thanks @sayantan1410 !
Just a small nit
@DhDeepLIT you were originally behind the request to improve our docs, can you please check if this PR provides enough info ? Thanks a lot ! https://deploy-preview-2448--pytorch-ignite-preview.netlify.app/metrics.html#attach-engine-api |
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.
Thanks @sayantan1410
Let's wait a bit for other comments if any and merge it later.
@vfdev-5 yeah, No problem !! |
Related to #2437
Added a sub heading to ignite.metrics, however I have not added its link to other pages.
Let me know the changes that I need to make.