Skip to content

Conversation

sayantan1410
Copy link
Contributor

@sayantan1410 sayantan1410 commented Feb 2, 2022

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.

  • Documentation is updated (if required)

@github-actions github-actions bot added the docs label Feb 2, 2022
Copy link
Collaborator

@vfdev-5 vfdev-5 left a 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

@sayantan1410
Copy link
Contributor Author

@vfdev-5 Hey, made the changes please check !!

@sayantan1410
Copy link
Contributor Author

@vfdev-5 Hey I have made some changes, let me know if they are alright !!

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Feb 7, 2022

@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 ?
For me there are few inconsistencies in the very beginning when you speak about accuracy (reader has directly the question why we talk about accuracy now). Code methods in text should be put in code-like ticks "`"

@sayantan1410
Copy link
Contributor Author

@vfdev-5 Hey, I tried to make it simpler and consistent, can you please check it once more.

Copy link
Collaborator

@vfdev-5 vfdev-5 left a 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

@vfdev-5
Copy link
Collaborator

vfdev-5 commented Feb 9, 2022

@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

Copy link
Collaborator

@vfdev-5 vfdev-5 left a 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 vfdev-5 requested a review from sdesrozis February 9, 2022 13:45
@sayantan1410
Copy link
Contributor Author

@vfdev-5 yeah, No problem !!

@vfdev-5 vfdev-5 enabled auto-merge (squash) February 14, 2022 11:25
@vfdev-5 vfdev-5 disabled auto-merge February 14, 2022 13:45
@vfdev-5 vfdev-5 merged commit e2fecf2 into pytorch:master Feb 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants