-
Notifications
You must be signed in to change notification settings - Fork 3.2k
[translation] adds docstrings and code snippets #17642
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
sdk/documenttranslation/azure-ai-documenttranslation/azure/ai/documenttranslation/_client.py
Outdated
Show resolved
Hide resolved
sdk/documenttranslation/azure-ai-documenttranslation/azure/ai/documenttranslation/_client.py
Outdated
Show resolved
Hide resolved
sdk/documenttranslation/azure-ai-documenttranslation/azure/ai/documenttranslation/_client.py
Outdated
Show resolved
Hide resolved
sdk/documenttranslation/azure-ai-documenttranslation/azure/ai/documenttranslation/_client.py
Outdated
Show resolved
Hide resolved
sdk/documenttranslation/azure-ai-documenttranslation/azure/ai/documenttranslation/_client.py
Show resolved
Hide resolved
sdk/documenttranslation/azure-ai-documenttranslation/azure/ai/documenttranslation/_models.py
Outdated
Show resolved
Hide resolved
sdk/documenttranslation/azure-ai-documenttranslation/azure/ai/documenttranslation/_models.py
Show resolved
Hide resolved
sdk/documenttranslation/azure-ai-documenttranslation/azure/ai/documenttranslation/_models.py
Outdated
Show resolved
Hide resolved
sdk/documenttranslation/azure-ai-documenttranslation/azure/ai/documenttranslation/_models.py
Show resolved
Hide resolved
iscai-msft
left a comment
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.
Really like your docstrings
| :rtype: ~azure.core.polling.ItemPaged[JobStatusResult] | ||
| """List all the submitted translation jobs under the Document Translation resource. | ||
| :keyword int top: Use top to indicate the total number of results you want |
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.
my understanding was you guys weren't going to expose top and skip for this release, I could be wrong though
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.
correct, will remove in future PR
| this is sorted by descending start time. | ||
| :keyword int results_per_page: Use results_per_page to indicate the maximum number | ||
| of results returned in a page. | ||
| :return: ItemPaged[:class:`~azure.ai.documenttranslation.JobStatusResult`] |
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.
is ItemPaged rendering correctly in sphinx
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.
Fixed. I think I had it like that to appease Apiview
| use the default_version for the file format returned from the | ||
| :func:`~DocumentTranslationClient.get_glossary_formats()` client method. | ||
| :keyword str storage_source: Storage Source. Default value: "AzureBlob". | ||
| Currently only "AzureBlob" is supported. |
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.
Do you need to add ivars here? Idk if users would actually access the properties, but you can
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.
Agree that it's unlikely a user would access them, but will add :ivars: to be more complete
| inner error with more descriptive details. | ||
| :ivar str status: Status for a job. | ||
| * `NotStarted` - the job has not begun yet. |
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.
A little out of this scope, but have you guys thought about making this an enum?
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.
This is only returned, never used as an input. If we made it an enum it would be purely for documentation purposes... last we had this conversation we decided that it should just be string.
Resolves #17046 #17047