Skip to content
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

[text analytics] analyze updates #16372

Closed
wants to merge 2 commits into from

Conversation

iscai-msft
Copy link
Contributor

fixes #16120

- Extract key phrases: [sample_extract_key_phrases.py][extract_key_phrases_sample] ([async version][extract_key_phrases_sample_async])
- Detect language: [sample_detect_language.py][detect_language_sample] ([async version][detect_language_sample_async])
- Healthcare Analysis: [sample_analyze_healthcare.py][analyze_healthcare_sample] ([async version][analyze_healthcare_sample_async])
- Batch Analysis: [sample_anayze.py][analyze_sample] ([async version][analyze_sample_async])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo in the filename (sample_anayze.py -> sample analyze.py)

~azure.ai.textanalytics.KeyPhraseExtractionTask, ~azure.ai.textanalytics.SentimentAnalysisTask]]
:type tasks: list[Union[~azure.ai.textanalytics.EntitiesRecognitionAction,
~azure.ai.textanalytics.PiiEntitiesRecognitionAction, ~azure.ai.textanalytics.EntityLinkingTask,
~azure.ai.textanalytics.KeyPhraseExtractionAction, ~azure.ai.textanalytics.SentimentAnalysisTask]]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think maybe this must've gotten left over from when we previously had planned to release Sentiment for v3.1-preview.3, so we should probably remove it for now.

~azure.ai.textanalytics.KeyPhraseExtractionTask, ~azure.ai.textanalytics.SentimentAnalysisTask]]
:type tasks: list[Union[~azure.ai.textanalytics.EntitiesRecognitionAction,
~azure.ai.textanalytics.PiiEntitiesRecognitionAction, ~azure.ai.textanalytics.EntityLinkingTask,
~azure.ai.textanalytics.KeyPhraseExtractionAction, ~azure.ai.textanalytics.SentimentAnalysisTask]]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as a previous comment for the sync client.

@@ -707,9 +707,9 @@ def begin_analyze( # type: ignore
list[dict[str, str]]
:param tasks: A list of tasks to include in the analysis. Each task object encapsulates the parameters
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also rename this to actions?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, below in the body of the method I think we will need to group the tasks by type in order to construct the JobManifest object. For some reason github won't let me add a comment directly on it, so check line 750.

@@ -709,9 +709,9 @@ async def begin_analyze( # type: ignore
list[dict[str, str]]
:param tasks: A list of tasks to include in the analysis. Each task object encapsulates the parameters
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as another comment for the sync client.

entities_recognition_tasks=[EntitiesRecognitionTask()],
pii_entities_recognition_tasks=[PiiEntitiesRecognitionTask()],
key_phrase_extraction_tasks=[KeyPhraseExtractionTask()]
entities_recognition_tasks=[EntitiesRecognitionAction()],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these were consolidated into a union type with name tasks above.

entities_recognition_tasks=[EntitiesRecognitionTask()],
pii_entities_recognition_tasks=[PiiEntitiesRecognitionTask()],
key_phrase_extraction_tasks=[KeyPhraseExtractionTask()]
entities_recognition_tasks=[EntitiesRecognitionAction()],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as the other sample.

@iscai-msft
Copy link
Contributor Author

closing because there were a bunch of formatting changes that my editor made. easier to start from scratch @abhahn thanks for starting to review!

@iscai-msft iscai-msft closed this Jan 29, 2021
ghost pushed a commit that referenced this pull request Mar 5, 2021
@iscai-msft iscai-msft deleted the analyze_updates branch May 13, 2021 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[text analytics] analyze endpoint changes
2 participants