[text analytics] add bing_id property to LinkedEntity class by iscai-msft · Pull Request #13446 · Azure/azure-sdk-for-python

iscai-msft · 2020-08-31T18:53:38Z

kristapratico · 2020-08-31T19:31:05Z

sdk/textanalytics/azure-ai-textanalytics/azure/ai/textanalytics/_models.py

    :ivar data_source: Data source used to extract entity linking,
        such as Wiki/Bing etc.
    :vartype data_source: str
+    :param str bing_id: Bing unique identifier of the recognized entity. Use in conjunction


Also should we add that sphinx directive for properties that are only available for certain versions?

Suggested change

:param str bing_id: Bing unique identifier of the recognized entity. Use in conjunction

:ivar str bing_id: Bing unique identifier of the recognized entity. Use in conjunction

good point, i'll add that, it might not look great but at least we'll have it. thanks!

We have other properties that should have this versionadded docstring too, have created an issue here trackcing that

kristapratico · 2020-08-31T19:37:12Z

sdk/textanalytics/azure-ai-textanalytics/tests/test_recognize_linked_entities.py

        client.recognize_linked_entities(["please don't fail"])
+
+    # currently only have this as playback since the dev endpoint is unreliable
+    @pytest.mark.playback_test_only


can we open an issue to remove playback marker when endpoint works reliably? (sorry for nit, I feel like these kinds of things are too easy to forget about)

np, good point here you go bb

kristapratico

LGTM

maririos · 2020-08-31T19:46:44Z

sdk/textanalytics/azure-ai-textanalytics/tests/test_recognize_linked_entities.py

+        "text_analytics_account": "https://cognitiveusw2dev.azure-api.net/"
+    })
+    def test_bing_id(self, client):
+        # make sure that the addition of the string_index_type kwarg for v3.1-preview.1 doesn't


comment needs to be updated

maririos · 2020-08-31T19:47:31Z

sdk/textanalytics/azure-ai-textanalytics/tests/test_recognize_linked_entities.py

+        "text_analytics_account_key": os.environ.get('AZURE_TEXT_ANALYTICS_KEY'),
+        "text_analytics_account": "https://cognitiveusw2dev.azure-api.net/"
+    })
+    def test_bing_id(self, client):


What are u testing here? not sure I understand the purpose of the test.

this one is just to check that I've correctly set the bing ID I get back from the service. I didn't tie it to the actual bing ID returned from the service though, I'm not sure how arbitrary that number is

maririos · 2020-08-31T19:49:26Z

sdk/textanalytics/azure-ai-textanalytics/tests/test_recognize_linked_entities.py

+        "text_analytics_account_key": os.environ.get('AZURE_TEXT_ANALYTICS_KEY'),
+        "text_analytics_account": "https://cognitiveusw2dev.azure-api.net/"
+    })
+    def test_bing_id(self, client):


do u know if the bingId property will always be populated?
worth adding a test checking that the property has content on it

It should be populated by the server for v3.1-preview.2 and up (it's not shown as an optional parameter). I'm confused by "worth adding a test checking that the property has content on it", I believe that's what I'm checking in this test

It might be me and not knowing ton of Python, but assert entity doesn't seem to be checking that bing_id has something on it. like it is not None or null, or empty

assert checks that it's not None and not an empty string

…into add_redacted_text * 'master' of https://github.com/Azure/azure-sdk-for-python: [text analytics] add bing_id property to LinkedEntity class (Azure#13446) fix typing for paging methods (Azure#13410) [text analytics] add domain_filter param (Azure#13451) fix issue Azure#11658 for is_valid_resource_id (Azure#11709) added create_table_if_not_exists method to table service client (Azure#13385) [ServiceBus] Test and failure improvements (Azure#13345) Proper encoding and decoding of source URLs - Fixes special characters in source URL issue (Azure#13275) Switch retry (Azure#13264) [ServiceBus] ServiceBusClient close spawned children (Azure#13077) fixing version issue by not overwriting the version with the semantic… (Azure#13411)

…into expose_parse_vault_id * 'master' of https://github.com/Azure/azure-sdk-for-python: (37 commits) [text analytics] add versionadded sphinx documentation (Azure#13450) [text analytics] add bing_id property to LinkedEntity class (Azure#13446) fix typing for paging methods (Azure#13410) [text analytics] add domain_filter param (Azure#13451) fix issue Azure#11658 for is_valid_resource_id (Azure#11709) added create_table_if_not_exists method to table service client (Azure#13385) [ServiceBus] Test and failure improvements (Azure#13345) Proper encoding and decoding of source URLs - Fixes special characters in source URL issue (Azure#13275) Switch retry (Azure#13264) [ServiceBus] ServiceBusClient close spawned children (Azure#13077) fixing version issue by not overwriting the version with the semantic… (Azure#13411) clean up reference and tests (Azure#13412) Consistent returns (Azure#13245) [text analytics] return None for offset and length for v3.0 (Azure#13382) edit all authentication files and add a test (Azure#13355) Add managed_identity_client_id argument to DefaultAzureCredential (Azure#13218) [text analytics] add string-index-type support (Azure#13378) [text analytics] fix error response if pii entities is called from v3.0 client (Azure#13383) Send spec (Azure#13143) Anomaly Detector 3.0.0b2 release (Azure#13351) ...

…into link_om_sample * 'master' of https://github.com/Azure/azure-sdk-for-python: (23 commits) Int32 serialization (Azure#13452) add output to opinion mining sample (Azure#13494) Add Document w/ Eng Sys Checks (Azure#13492) update version (Azure#13495) Remove resources post test (Azure#13379) bing_id -> bing_entity_search_api_id (Azure#13491) [EventGrid] Read me + improve docstrings (Azure#13484) Build AuthenticationRecords from ADFS identity tokens (Azure#13341) Support Subject Name/Issuer authentication (Azure#13350) Add KeyVaultAccessControlClient for data plane RBAC (Azure#13372) [text analytics] Add redacted_text (Azure#13449) add python sdk sample (Azure#13338) [text analytics] add versionadded sphinx documentation (Azure#13450) [text analytics] add bing_id property to LinkedEntity class (Azure#13446) fix typing for paging methods (Azure#13410) [text analytics] add domain_filter param (Azure#13451) fix issue Azure#11658 for is_valid_resource_id (Azure#11709) added create_table_if_not_exists method to table service client (Azure#13385) [ServiceBus] Test and failure improvements (Azure#13345) Proper encoding and decoding of source URLs - Fixes special characters in source URL issue (Azure#13275) ...

Add the march public preview API version of extended location (Azure#13446) * Add the march public preview API version of extended location * Tweak: remove references to iot * Address comments from Phoenix by removing 500 references and adding back the default version tag * Revert privatepreview changes * Tweak: remove "500" from the swagger itself * Tweak: remove dash from readme * fix python pipeline * Tweak: run prettier against the repo folder * Tweak: remove 404 from customlocations.json and CustomLocationsGet.json * Tweak: csharp README sdk output folder path to fix downstream generation issue Co-authored-by: 00Kai0 <[email protected]>

iscai-msft added 4 commits August 31, 2020 11:56

add bing_id property

2bbcf2b

add repr test

871d70f

fix pylint

0541196

add playback only tests for bing id

7b8d38f

iscai-msft requested a review from kristapratico August 31, 2020 18:53

ghost added the Cognitive - Text Analytics label Aug 31, 2020

iscai-msft requested review from deyaaeldeen, maririos and mssfang August 31, 2020 18:53

hardcode endpoint in tests

3657828

kristapratico reviewed Aug 31, 2020

View reviewed changes

fix bing_id docstring

1b37ab9

kristapratico reviewed Aug 31, 2020

View reviewed changes

kristapratico previously approved these changes Aug 31, 2020

View reviewed changes

maririos reviewed Aug 31, 2020

View reviewed changes

fix copy paste mistakes in tests

23d9197

iscai-msft requested a review from kristapratico August 31, 2020 20:21

iscai-msft dismissed kristapratico’s stale review via 23d9197 August 31, 2020 20:21

iscai-msft requested a review from maririos August 31, 2020 20:21

kristapratico previously approved these changes Aug 31, 2020

View reviewed changes

actually verify bing_id property

9f26362

iscai-msft requested a review from kristapratico August 31, 2020 21:22

iscai-msft dismissed kristapratico’s stale review via 9f26362 August 31, 2020 21:22

kristapratico approved these changes Aug 31, 2020

View reviewed changes

maririos approved these changes Aug 31, 2020

View reviewed changes

mssfang approved these changes Sep 1, 2020

View reviewed changes

iscai-msft merged commit 9b1b9ec into Azure:master Sep 1, 2020

iscai-msft deleted the add_bing_id branch September 1, 2020 14:53

	:param str bing_id: Bing unique identifier of the recognized entity. Use in conjunction
	:ivar str bing_id: Bing unique identifier of the recognized entity. Use in conjunction

Conversation

iscai-msft commented Aug 31, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kristapratico left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iscai-msft Aug 31, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

iscai-msft Aug 31, 2020 •

edited

Loading