Skip to content

[text analytics] change return type of analyze actions to list of list#18994

Merged
kristapratico merged 5 commits intoAzure:masterfrom
iscai-msft:update_analyze_output
Jun 14, 2021
Merged

[text analytics] change return type of analyze actions to list of list#18994
kristapratico merged 5 commits intoAzure:masterfrom
iscai-msft:update_analyze_output

Conversation

@iscai-msft
Copy link
Copy Markdown
Contributor

@iscai-msft iscai-msft commented May 27, 2021

Instead of adding a new class AnalyzeActionsResult, we're going to reuse the current action result classes, i.e. RecognizeEntitiesResult.

Now, a call with documents ["Hello", "world"] and actions [RecognizeEntitiesAction(), AnalyzeSentimentAction()] will get you
[
[RecognizeEntitiesResult of "Hello", AnalyzeSentimentResult of "Hello"],
[RecognizeEntitiesResult of "world", AnalyzeSentimentResult of "world"],
]

This is more consistent with the return types of the normal operations, and allows us to use the models we currently have

@iscai-msft iscai-msft requested a review from johanste May 27, 2021 22:34
kristapratico
kristapratico approved these changes Jun 1, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@kristapratico kristapratico left a comment

Choose a reason for hiding this comment

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

nits

johanste
johanste reviewed Jun 1, 2021
View reviewed changes
sdk/textanalytics/azure-ai-textanalytics/README.md Outdated
for doc, document_results in zip(documents, pages):
print("\nDocument text: {}".format(doc))
recognize_entities_result = document_results[0]
assert not recognize_entities_result.is_error
Copy link
Copy Markdown
Member

@johanste johanste Jun 1, 2021

Choose a reason for hiding this comment

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

This is probably not a good use of an assert. Any document could have failed for a transient reason, so you are not really in a position to say that this didn't fail. I would print a "failed" message instead...

Copy link
Copy Markdown
Member

@johanste johanste Jun 1, 2021

Choose a reason for hiding this comment

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

Actually, scratch that, this is in the readme.md. It is a bit long for the readme already. I don't think we need to show all types of actions in the readme example inline. It definitely won't scale as new action types comes along. Can we limit this to two?

@iscai-msft iscai-msft requested a review from johanste June 1, 2021 17:22
iscai-msft added 2 commits June 1, 2021 13:25
@iscai-msft
limit # of action calls in readme example
86f8294
@iscai-msft
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-python …
f192e29 
…into update_analyze_output

* 'master' of https://github.com/Azure/azure-sdk-for-python: (123 commits)
  [text analytics] Fix some issues i've found (Azure#19081)
  [Key Vault] Add API version 7.2 for administration (Azure#18997)
  hide to_analyze_request (Azure#19079)
  Add environment variable for redirecting IMDS token requests (Azure#18967)
  [translation] poller design (Azure#19041)
  [AutoRelease] t2-resourcemover-2021-05-21-54304(wave4) (Azure#18849)
  [AutoRelease] t2-logz-2021-06-02-49354 (Azure#19035)
  prepare pipeline (Azure#19062)
  [AutoRelease] t2-recoveryservicesbackup-2021-05-26-33193 (Azure#18953)
  Get rid of DataFeedOptions (Azure#19054)
  [AutoRelease] t2-batchai-2021-06-02-38609 (Azure#19036)
  [appconfig] new gen code (Azure#18859)
  [Tables] mypy (Azure#19001)
  fix misleading url (Azure#19044)
  Create py.typed (Azure#19052)
  add override of opentelemetry-api to 1.3.0 (Azure#19048)
  Prepare for June Release (Azure#19043)
  [Key Vault] Add API version 7.2 for keys (Azure#18586)
  some misc fixes. (Azure#19024)
  [AppConfig] pre release (Azure#19027)
  ...
@kristapratico kristapratico mentioned this pull request Jun 10, 2021
Closed
5 tasks
@kristapratico kristapratico merged commit 407d0b7 into Azure:master Jun 14, 2021
iscai-msft added a commit that referenced this pull request Jun 16, 2021
@iscai-msft
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-python …
b03039a 
…into protocol_base

* 'master' of https://github.com/Azure/azure-sdk-for-python: (229 commits)
  Sync eng/common directory with azure-sdk-tools for PR 1688 (#19272)
  update all ubuntu vmImage to 20.04 (#19227)
  add base class for feedback (#19265)
  Enable tests for integration samples (#18531)
  docs: fix a few simple typos (#19127)
  Increment package version after release of azure-eventgrid (#19197)
  Increment package version after release of azure-monitor-query (#19208)
  earliest versions of communication identity tests are set up improperly. skip 1.0.0 in regression testing (#19258)
  Run mypy in azure-keyvault-administration CI (#19246)
  [text analytics] change return type of analyze actions to list of list (#18994)
  Increment package version after release of azure-data-tables (#19192)
  suppress min number less than 14 (#19230)
  [Key Vault] Address administration feedback (#19099)
  [textanalytics] remove warning tests (#19217)
  [translation] enable samples to run in CI (#19190)
  update custom_granularity_value to be more than 300 (#19215)
  Increment package version after release of azure-security-attestation (#19212)
  Enabling Before/After Test Customization for Standard CI Pipelines (#19187)
  bump sphinx version to resolve the weird issue with the caption not getting the correct class (#19188)
  [AutoRelease] t2-agrifood-2021-06-09-15946 (#19174)
  ...
iscai-msft added a commit to iscai-msft/azure-sdk-for-python that referenced this pull request Jun 17, 2021
@iscai-msft
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-python …
fb692df 
…into get_testserver_working

* 'master' of https://github.com/Azure/azure-sdk-for-python: (55 commits)
  [AppConfig] mypy fixes (Azure#18858)
  [translation] renames (Azure#19285)
  [Key Vault] Update metadata for release (Azure#19286)
  [AppConfig] enable samples (Azure#18857)
  KeyVaultBackupOperation -> KeyVaultBackupResult (Azure#19284)
  renaming (Azure#19280)
  [Key Vault] Custom polling method for admin backup client (Azure#19204)
  [textanalytics] add ARM template + run samples in CI (Azure#19270)
  removed example from description (Azure#18781)
  [Key Vault] Update READMEs with RBAC information (Azure#19275)
  Sync eng/common directory with azure-sdk-tools for PR 1688 (Azure#19272)
  update all ubuntu vmImage to 20.04 (Azure#19227)
  add base class for feedback (Azure#19265)
  Enable tests for integration samples (Azure#18531)
  docs: fix a few simple typos (Azure#19127)
  Increment package version after release of azure-eventgrid (Azure#19197)
  Increment package version after release of azure-monitor-query (Azure#19208)
  earliest versions of communication identity tests are set up improperly. skip 1.0.0 in regression testing (Azure#19258)
  Run mypy in azure-keyvault-administration CI (Azure#19246)
  [text analytics] change return type of analyze actions to list of list (Azure#18994)
  ...
@iscai-msft iscai-msft deleted the update_analyze_output branch September 10, 2024 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@johanste johanste johanste approved these changes

@abhahn abhahn Awaiting requested review from abhahn

+1 more reviewer

@kristapratico kristapratico kristapratico approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Cognitive - Text Analytics

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@iscai-msft @johanste @kristapratico