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.

Sign up for GitHub

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

Jump to bottom

[text analytics] Fix some issues i've found #19081

Merged
merged 7 commits into from
Jun 3, 2021
Merged

[text analytics] Fix some issues i've found #19081

merged 7 commits into from
Jun 3, 2021

Conversation

iscai-msft
Copy link
Contributor

  1. Remove custom pager: we don't need it
  2. Fix AnalyzeHealthcareEntitiesResult's statistics property to be the correct type (TextDocumentStatistics)
  3. Remove RequestStatistics (it was a duplicate of TextDocumentBatchStatistics)
  4. Fix names of the LRO pollers and polling methods, and fix the typing and documentation to refer to the custom pollers instead of LROPoller
  5. Make typehints for async functions not commented
  6. Add documentation strings for the metadata exposed on the custom LRO pollers

fixes #19014, #16668, #16483

@iscai-msft
add stats for analyze policies
dae01b1
@iscai-msft
remove deserializer and custom ItemPaged
786883c
@iscai-msft
rename AnalyzeHealthcareEntitiesResultItem -> AnalyzeHealthcareEntiti…
38af014 
…esResult
@iscai-msft
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-python …
a5f8a55 
…into fix_paging_types

* 'master' of https://github.com/Azure/azure-sdk-for-python: (37 commits)
  [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)
  [ACR] prerelease (Azure#19028)
  Add sdk/core to triggers and pr (Azure#19023)
  Event grid 4.3.0 regen code  (Azure#19025)
  Handle 6 decimal places cloud event (Azure#19019)
  ...
@iscai-msft
move custom lros, rename, and add docstrings
4021856
@iscai-msft
fix tests
8d50d9a
@iscai-msft iscai-msft requested a review from abhahn as a code owner June 3, 2021 18:31
kristapratico
kristapratico approved these changes Jun 3, 2021
View reviewed changes
sdk/textanalytics/azure-ai-textanalytics/azure/ai/textanalytics/_lro.py
class AnalyzeHealthcareEntitiesLROPoller(LROPoller):
class AnalyzeHealthcareEntitiesLROPoller(LROPoller, Generic[PollingReturnType]):

def polling_method(self):
Copy link
Member

Choose a reason for hiding this comment

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

do users need to access this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, this was for typing basically. It's inherited from LROPoller, but since I added metadata on my pollers + polling methods (metadata on polling methods isn't really exposed to users, but is used to pass info to the poller), I'm using the polling method to let mypy know that "yep, my poller / polling method really has these metadata attributes"

sdk/textanalytics/azure-ai-textanalytics/tests/test_analyze_healthcare.py
for idx, resp in enumerate(response):
self.assertEqual(resp.id, expected_order[idx])
assert resp.id == expected_order[idx]
if resp.is_error:
Copy link
Member

Choose a reason for hiding this comment

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

are we expecting errors in this test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, one of them is an empty doc

@iscai-msft iscai-msft merged commit 0282a23 into Azure:master Jun 3, 2021
@iscai-msft iscai-msft deleted the fix_paging_types branch June 3, 2021 20:31
iscai-msft added a commit to iscai-msft/azure-sdk-for-python that referenced this pull request Jun 3, 2021
@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)
  ...
azure-sdk pushed a commit to azure-sdk/azure-sdk-for-python that referenced this pull request Jun 2, 2022
@cvvz @wenxuan0923
@bingosummer @JatinSanghvi @orparnes
CodeGen from PR 19262 in Azure/azure-rest-api-specs
1dbfecb 
Review request for Microsoft.ContainerService to add version 2022-05-02-preview (Azure#19262)

* Adds base for updating Microsoft.ContainerService from version preview/2022-04-02-preview to version 2022-05-02-preview

* Updates readme

* Updates API version in new specs and examples

* update readmes (Azure#19081)

* add blob csi driver into 0502preview (Azure#19095)

Co-authored-by: weizhichen <[email protected]>

* Add read only field for `currentOrchestratorVersion` (Azure#19107)

* currentOrchestratorVersion is read-only property

* Only apply change to 2022-05-02-preview

* Add keyVaultNetworkAccess and keyVaultResourceId into securityProfile.azureKeyVaultKms to support key vault with private link (Azure#19086)

* Add keyVaultNetworkAccess and keyVaultResourceId into securityProfile.azureKeyVaultKms to support key vault with private link

* fix

* Add KEDA configuration options (Azure#19153)

* 2022-05-02-preview defender updates (Azure#19172)

* 2022-05-02-preview defender updates

* update example

* Typo

* Update sample

* typo

* another typo

Co-authored-by: weizhi <[email protected]>
Co-authored-by: weizhichen <[email protected]>
Co-authored-by: Thalia Wang <[email protected]>
Co-authored-by: Bin Xia <[email protected]>
Co-authored-by: Jatin Sanghvi <[email protected]>
Co-authored-by: Or Parnes <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kristapratico kristapratico kristapratico approved these changes

@abhahn abhahn Awaiting requested review from abhahn

Assignees
No one assigned
Labels
Cognitive - Text Analytics
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[text analytics] have async typehints be not commented
2 participants
@iscai-msft @kristapratico