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

Add method-specific request options to TextAnalyticsClient #20049

Merged
merged 6 commits into from
Apr 2, 2021
Merged

Add method-specific request options to TextAnalyticsClient #20049

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
36ceb14
Add method-specific request options to TextAnalyticsClient
heaths Apr 1, 2021
dc02f95
Assert options are no different
heaths Apr 1, 2021
bfa3444
Fix build issues
heaths Apr 2, 2021
9c71b4a
Use RecognizeEntitiesOptions in new overloads
heaths Apr 2, 2021
c59a4e1
Update samples
heaths Apr 2, 2021
a977df6
Add overloads for ExtractKeyPhrasesBatch(Async)
heaths Apr 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions sdk/textanalytics/Azure.AI.TextAnalytics.Protocol/demo/DemoApp.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>net5.0</TargetFramework>
<IsPackable>false</IsPackable>
Copy link
Member

Choose a reason for hiding this comment

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

FYI @ellismg

Copy link
Member Author

Choose a reason for hiding this comment

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

Is that just to let him know it's possible, or was this actually supposed to be up on nuget.org? I'm surprised this hasn't failed in the past. Though, I see from git blame @ellismg added it.

If a "demo" directory sibling to "src" and "tests" is an emerging standard, we can update the build targets to add <IsPackable>false</IsPackable> by default - as well as a few other tweaks to treat them more like tests or even samples. But, I would add that with Key Vault I was trying to start a standard on a "samples" subdirectory sibling to those directories or even the service directory itself, e.g. sdk/keyvault/samples that contain some samples not specific to a package.

Copy link
Member

Choose a reason for hiding this comment

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

as FYI as he owns TA.Protocol

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for making the change here, as the intention was not to create anything that should be published to NuGet.org. The "demo" pattern isn't something I think we want to push long term, it was mainly there because having a small console program that we could use to test out how the API felt was useful and it helped us as a way to prototype some stuff.

I'm happy to just go delete this now if it's causing additional problems or confusion. We are not really using TA to prototype the low-level client work now (we have some other services we've been focused on) so I'm happy to remove the entire .Protocol folder.

Copy link
Member Author

Choose a reason for hiding this comment

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

If you don't feel it worth keeping, I'm all for reducing points of confusion.

</PropertyGroup>

<ItemGroup>
Expand Down
5 changes: 4 additions & 1 deletion sdk/textanalytics/Azure.AI.TextAnalytics/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
# Release History

## 5.1.0-beta.6 (Unreleased)

### New features
- Add overloads to `ExtractKeyPhrasesBatch` and `ExtractKeyPhrasesBatchAsync` to on `TextAnalyticsClient` to accept `ExtractKeyPhrasesOptions` and hid the previous methods (non-breaking change).
- Add overloads to `RecognizeEntitiesBatch` and `RecognizeEntitiesBatchAsync` to on `TextAnalyticsClient` to accept `RecognizeEntitiesOptions` and hid the previous methods (non-breaking change).
- Add overloads to `RecognizeLinkedEntitiesBatch` and `RecognizeLinkedEntitiesBatch` to on `TextAnalyticsClient` to accept `RecognizeLinkedEntitiesOptions` and hid the previous methods (non-breaking change).

## 5.1.0-beta.5 (2021-03-09)
### New features
Expand Down
60 changes: 48 additions & 12 deletions sdk/textanalytics/Azure.AI.TextAnalytics/api/Azure.AI.TextAnalytics.netstandard2.0.cs
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion sdk/textanalytics/Azure.AI.TextAnalytics/samples/Sample3_ExtractKeyPhrases.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ var documents = new List<TextDocumentInput>
new TextDocumentInput("4", string.Empty)
};

var options = new TextAnalyticsRequestOptions { IncludeStatistics = true };
var options = new ExtractKeyPhrasesOptions { IncludeStatistics = true };
Response<ExtractKeyPhrasesResultCollection> response = client.ExtractKeyPhrasesBatch(documents, options);
ExtractKeyPhrasesResultCollection keyPhrasesInDocuments = response.Value;

Expand Down
2 changes: 1 addition & 1 deletion sdk/textanalytics/Azure.AI.TextAnalytics/samples/Sample4_RecognizeEntities.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ var documents = new List<TextDocumentInput>
new TextDocumentInput("4", string.Empty)
};

var options = new TextAnalyticsRequestOptions { IncludeStatistics = true };
var options = new RecognizeEntitiesOptions { IncludeStatistics = true };
Response<RecognizeEntitiesResultCollection> response = client.RecognizeEntitiesBatch(documents, options);
RecognizeEntitiesResultCollection entitiesInDocuments = response.Value;

Expand Down
2 changes: 1 addition & 1 deletion ...textanalytics/Azure.AI.TextAnalytics/samples/Sample6_RecognizeLinkedEntities.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ var documents = new List<TextDocumentInput>
new TextDocumentInput("4", string.Empty)
};

var options = new TextAnalyticsRequestOptions { IncludeStatistics = true };
var options = new RecognizeLinkedEntitiesOptions { IncludeStatistics = true };
Response<RecognizeLinkedEntitiesResultCollection> response = client.RecognizeLinkedEntitiesBatch(documents, options);
RecognizeLinkedEntitiesResultCollection entitiesPerDocuments = response.Value;

Expand Down
516 changes: 492 additions & 24 deletions sdk/textanalytics/Azure.AI.TextAnalytics/src/TextAnalyticsClient.cs
View file
Open in desktop

Large diffs are not rendered by default.

54 changes: 52 additions & 2 deletions sdk/textanalytics/Azure.AI.TextAnalytics/tests/ExtractKeyPhrasesTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,14 +125,39 @@ public async Task ExtractKeyPhrasesBatchConvenienceTest()
[Test]
public async Task ExtractKeyPhrasesBatchConvenienceWithStatisticsTest()
{
var options = new TextAnalyticsRequestOptions { IncludeStatistics = true };
TextAnalyticsClient client = GetClient();
var documents = batchConvenienceDocuments;

ExtractKeyPhrasesResultCollection results = await client.ExtractKeyPhrasesBatchAsync(documents, "en", new TextAnalyticsRequestOptions { IncludeStatistics = true });
ExtractKeyPhrasesResultCollection results = await client.ExtractKeyPhrasesBatchAsync(documents, "en", options);

ValidateBatchDocumentsResult(results, 3, includeStatistics: true);

Assert.AreEqual(documents.Count, results.Statistics.DocumentCount);

// Assert the options classes since overloads were added and the original now instantiates a RecognizeEntitiesOptions.
Assert.IsTrue(options.IncludeStatistics);
Assert.IsNull(options.ModelVersion);
Assert.AreEqual(StringIndexType.Utf16CodeUnit, options.StringIndexType);
}

[Test]
public async Task ExtractKeyPhrasesBatchConvenienceWithExtractKeyPhrasesOptionsStatisticsTest()
{
var options = new ExtractKeyPhrasesOptions { IncludeStatistics = true };
TextAnalyticsClient client = GetClient();
var documents = batchConvenienceDocuments;

ExtractKeyPhrasesResultCollection results = await client.ExtractKeyPhrasesBatchAsync(documents, "en", options);

ValidateBatchDocumentsResult(results, 3, includeStatistics: true);

Assert.AreEqual(documents.Count, results.Statistics.DocumentCount);

// Assert the options classes since overloads were added and the original now instantiates a RecognizeEntitiesOptions.
Assert.IsTrue(options.IncludeStatistics);
Assert.IsNull(options.ModelVersion);
Assert.AreEqual(StringIndexType.Utf16CodeUnit, options.StringIndexType);
}

[Test]
Expand All @@ -149,14 +174,39 @@ public async Task ExtractKeyPhrasesBatchTest()
[Test]
public async Task ExtractKeyPhrasesBatchWithSatisticsTest()
{
var options = new TextAnalyticsRequestOptions { IncludeStatistics = true };
TextAnalyticsClient client = GetClient();
List<TextDocumentInput> documents = batchDocuments;

ExtractKeyPhrasesResultCollection results = await client.ExtractKeyPhrasesBatchAsync(documents, new TextAnalyticsRequestOptions { IncludeStatistics = true });
ExtractKeyPhrasesResultCollection results = await client.ExtractKeyPhrasesBatchAsync(documents, options);

ValidateBatchDocumentsResult(results, 3, includeStatistics: true);

Assert.AreEqual(documents.Count, results.Statistics.DocumentCount);

// Assert the options classes since overloads were added and the original now instantiates a RecognizeEntitiesOptions.
Assert.IsTrue(options.IncludeStatistics);
Assert.IsNull(options.ModelVersion);
Assert.AreEqual(StringIndexType.Utf16CodeUnit, options.StringIndexType);
}

[Test]
public async Task ExtractKeyPhrasesBatchWithExtractKeyPhrasesOptionsSatisticsTest()
{
var options = new ExtractKeyPhrasesOptions { IncludeStatistics = true };
TextAnalyticsClient client = GetClient();
List<TextDocumentInput> documents = batchDocuments;

ExtractKeyPhrasesResultCollection results = await client.ExtractKeyPhrasesBatchAsync(documents, options);

ValidateBatchDocumentsResult(results, 3, includeStatistics: true);

Assert.AreEqual(documents.Count, results.Statistics.DocumentCount);

// Assert the options classes since overloads were added and the original now instantiates a RecognizeEntitiesOptions.
Assert.IsTrue(options.IncludeStatistics);
Assert.IsNull(options.ModelVersion);
Assert.AreEqual(StringIndexType.Utf16CodeUnit, options.StringIndexType);
}

[Test]
Expand Down
92 changes: 89 additions & 3 deletions sdk/textanalytics/Azure.AI.TextAnalytics/tests/RecognizeEntitiesTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,11 @@ public async Task RecognizeEntitiesWithLanguageTest()
[Test]
public async Task RecognizeEntitiesWithSubCategoryTest()
{
TextAnalyticsRequestOptions options = new TextAnalyticsRequestOptions() { ModelVersion = "2020-04-01" };
TextAnalyticsClient client = GetClient();
string document = "I had a wonderful trip to Seattle last week.";

RecognizeEntitiesResultCollection result = await client.RecognizeEntitiesBatchAsync(new List<string>() { document }, options: new TextAnalyticsRequestOptions() { ModelVersion = "2020-04-01" } );
RecognizeEntitiesResultCollection result = await client.RecognizeEntitiesBatchAsync(new List<string>() { document }, options: options );

var documentResult = result.FirstOrDefault();
Assert.IsFalse(documentResult.HasError);
Expand All @@ -93,6 +94,37 @@ public async Task RecognizeEntitiesWithSubCategoryTest()
if (entity.Text == "last week")
Assert.AreEqual("DateRange", entity.SubCategory);
}

// Assert the options classes since overloads were added and the original now instantiates a RecognizeEntitiesOptions.
maririos marked this conversation as resolved.
Show resolved Hide resolved
Assert.IsFalse(options.IncludeStatistics);
Assert.AreEqual("2020-04-01", options.ModelVersion);
Assert.AreEqual(StringIndexType.Utf16CodeUnit, options.StringIndexType);
}

[Test]
public async Task RecognizeEntitiesWithRecognizeEntitiesOptionsSubCategoryTest()
{
RecognizeEntitiesOptions options = new RecognizeEntitiesOptions() { ModelVersion = "2020-04-01" };
TextAnalyticsClient client = GetClient();
string document = "I had a wonderful trip to Seattle last week.";

RecognizeEntitiesResultCollection result = await client.RecognizeEntitiesBatchAsync(new List<string>() { document }, options: options );

var documentResult = result.FirstOrDefault();
Assert.IsFalse(documentResult.HasError);

Assert.GreaterOrEqual(documentResult.Entities.Count, 3);

foreach (CategorizedEntity entity in documentResult.Entities)
{
if (entity.Text == "last week")
Assert.AreEqual("DateRange", entity.SubCategory);
}

// Assert the options classes since overloads were added and the original now instantiates a RecognizeEntitiesOptions.
Assert.IsFalse(options.IncludeStatistics);
Assert.AreEqual("2020-04-01", options.ModelVersion);
Assert.AreEqual(StringIndexType.Utf16CodeUnit, options.StringIndexType);
}

[Test]
Expand Down Expand Up @@ -155,8 +187,30 @@ public async Task RecognizeEntitiesBatchConvenienceTest()
[Test]
public async Task RecognizeEntitiesBatchConvenienceWithStatisticsTest()
{
TextAnalyticsRequestOptions options = new TextAnalyticsRequestOptions { IncludeStatistics = true };
TextAnalyticsClient client = GetClient();
RecognizeEntitiesResultCollection results = await client.RecognizeEntitiesBatchAsync(s_batchConvenienceDocuments, "en", options);

var expectedOutput = new Dictionary<string, List<string>>()
{
{ "0", s_document1ExpectedOutput },
{ "1", s_document2ExpectedOutput },
};

ValidateBatchDocumentsResult(results, expectedOutput, includeStatistics: true);

// Assert the options classes since overloads were added and the original now instantiates a RecognizeEntitiesOptions.
Assert.IsTrue(options.IncludeStatistics);
Assert.IsNull(options.ModelVersion);
Assert.AreEqual(StringIndexType.Utf16CodeUnit, options.StringIndexType);
}

[Test]
public async Task RecognizeEntitiesBatchConvenienceWithRecognizeEntitiesOptionsStatisticsTest()
{
RecognizeEntitiesOptions options = new RecognizeEntitiesOptions { IncludeStatistics = true };
TextAnalyticsClient client = GetClient();
RecognizeEntitiesResultCollection results = await client.RecognizeEntitiesBatchAsync(s_batchConvenienceDocuments, "en", new TextAnalyticsRequestOptions { IncludeStatistics = true });
RecognizeEntitiesResultCollection results = await client.RecognizeEntitiesBatchAsync(s_batchConvenienceDocuments, "en", options);

var expectedOutput = new Dictionary<string, List<string>>()
{
Expand All @@ -165,6 +219,11 @@ public async Task RecognizeEntitiesBatchConvenienceWithStatisticsTest()
};

ValidateBatchDocumentsResult(results, expectedOutput, includeStatistics: true);

// Assert the options classes since overloads were added and the original now instantiates a RecognizeEntitiesOptions.
Assert.IsTrue(options.IncludeStatistics);
Assert.IsNull(options.ModelVersion);
Assert.AreEqual(StringIndexType.Utf16CodeUnit, options.StringIndexType);
}

[Test]
Expand All @@ -185,8 +244,9 @@ public async Task RecognizeEntitiesBatchTest()
[Test]
public async Task RecognizeEntitiesBatchWithStatisticsTest()
{
TextAnalyticsRequestOptions options = new TextAnalyticsRequestOptions { IncludeStatistics = true };
TextAnalyticsClient client = GetClient();
RecognizeEntitiesResultCollection results = await client.RecognizeEntitiesBatchAsync(s_batchDocuments, new TextAnalyticsRequestOptions { IncludeStatistics = true });
RecognizeEntitiesResultCollection results = await client.RecognizeEntitiesBatchAsync(s_batchDocuments, options);

var expectedOutput = new Dictionary<string, List<string>>()
{
Expand All @@ -195,6 +255,32 @@ public async Task RecognizeEntitiesBatchWithStatisticsTest()
};

ValidateBatchDocumentsResult(results, expectedOutput, includeStatistics: true);

// Assert the options classes since overloads were added and the original now instantiates a RecognizeEntitiesOptions.
Assert.IsTrue(options.IncludeStatistics);
Assert.IsNull(options.ModelVersion);
Assert.AreEqual(StringIndexType.Utf16CodeUnit, options.StringIndexType);
}

[Test]
public async Task RecognizeEntitiesBatchWithRecognizeEntitiesOptionsStatisticsTest()
{
RecognizeEntitiesOptions options = new RecognizeEntitiesOptions { IncludeStatistics = true };
TextAnalyticsClient client = GetClient();
RecognizeEntitiesResultCollection results = await client.RecognizeEntitiesBatchAsync(s_batchDocuments, options);

var expectedOutput = new Dictionary<string, List<string>>()
{
{ "1", s_document1ExpectedOutput },
{ "2", s_document1ExpectedOutput },
};

ValidateBatchDocumentsResult(results, expectedOutput, includeStatistics: true);

// Assert the options classes since overloads were added and the original now instantiates a RecognizeEntitiesOptions.
Assert.IsTrue(options.IncludeStatistics);
Assert.IsNull(options.ModelVersion);
Assert.AreEqual(StringIndexType.Utf16CodeUnit, options.StringIndexType);
}

[Test]
Expand Down
58 changes: 56 additions & 2 deletions sdk/textanalytics/Azure.AI.TextAnalytics/tests/RecognizeLinkedEntitiesTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,9 @@ public async Task RecognizeLinkedEntitiesBatchConvenienceTest()
[Test]
public async Task RecognizeLinkedEntitiesBatchConvenienceWithStatisticsTest()
{
TextAnalyticsRequestOptions options = new TextAnalyticsRequestOptions { IncludeStatistics = true };
TextAnalyticsClient client = GetClient();
RecognizeLinkedEntitiesResultCollection results = await client.RecognizeLinkedEntitiesBatchAsync(s_batchConvenienceDocuments, "en", new TextAnalyticsRequestOptions { IncludeStatistics = true });
RecognizeLinkedEntitiesResultCollection results = await client.RecognizeLinkedEntitiesBatchAsync(s_batchConvenienceDocuments, "en", options);

var expectedOutput = new Dictionary<string, List<string>>()
{
Expand All @@ -146,6 +147,32 @@ public async Task RecognizeLinkedEntitiesBatchConvenienceWithStatisticsTest()
};

ValidateBatchDocumentsResult(results, expectedOutput, includeStatistics: true);

// Assert the options classes since overloads were added and the original now instantiates a RecognizeLinkedEntitiesOptions.
Assert.IsTrue(options.IncludeStatistics);
Assert.IsNull(options.ModelVersion);
Assert.AreEqual(StringIndexType.Utf16CodeUnit, options.StringIndexType);
}

[Test]
public async Task RecognizeLinkedEntitiesBatchConvenienceWithRecognizeLinkedEntitiesOptionsStatisticsTest()
{
RecognizeLinkedEntitiesOptions options = new RecognizeLinkedEntitiesOptions { IncludeStatistics = true };
TextAnalyticsClient client = GetClient();
RecognizeLinkedEntitiesResultCollection results = await client.RecognizeLinkedEntitiesBatchAsync(s_batchConvenienceDocuments, "en", options);

var expectedOutput = new Dictionary<string, List<string>>()
{
{ "0", s_document1ExpectedOutput },
{ "1", s_document2ExpectedOutput },
};

ValidateBatchDocumentsResult(results, expectedOutput, includeStatistics: true);

// Assert the options classes since overloads were added and the original now instantiates a RecognizeLinkedEntitiesOptions.
Assert.IsTrue(options.IncludeStatistics);
Assert.IsNull(options.ModelVersion);
Assert.AreEqual(StringIndexType.Utf16CodeUnit, options.StringIndexType);
}

[Test]
Expand All @@ -166,8 +193,9 @@ public async Task RecognizeLinkedEntitiesBatchTest()
[Test]
public async Task RecognizeLinkedEntitiesBatchWithStatisticsTest()
{
TextAnalyticsRequestOptions options = new TextAnalyticsRequestOptions { IncludeStatistics = true };
TextAnalyticsClient client = GetClient();
RecognizeLinkedEntitiesResultCollection results = await client.RecognizeLinkedEntitiesBatchAsync(s_batchDocuments, new TextAnalyticsRequestOptions { IncludeStatistics = true });
RecognizeLinkedEntitiesResultCollection results = await client.RecognizeLinkedEntitiesBatchAsync(s_batchDocuments, options);

var expectedOutput = new Dictionary<string, List<string>>()
{
Expand All @@ -176,6 +204,32 @@ public async Task RecognizeLinkedEntitiesBatchWithStatisticsTest()
};

ValidateBatchDocumentsResult(results, expectedOutput, includeStatistics: true);

// Assert the options classes since overloads were added and the original now instantiates a RecognizeLinkedEntitiesOptions.
Assert.IsTrue(options.IncludeStatistics);
Assert.IsNull(options.ModelVersion);
Assert.AreEqual(StringIndexType.Utf16CodeUnit, options.StringIndexType);
}

[Test]
public async Task RecognizeLinkedEntitiesBatchWithRecognizeLinkedEntitiesOptionsStatisticsTest()
{
RecognizeLinkedEntitiesOptions options = new RecognizeLinkedEntitiesOptions { IncludeStatistics = true };
TextAnalyticsClient client = GetClient();
RecognizeLinkedEntitiesResultCollection results = await client.RecognizeLinkedEntitiesBatchAsync(s_batchDocuments, options);

var expectedOutput = new Dictionary<string, List<string>>()
{
{ "1", s_document1ExpectedOutput },
{ "3", s_document1ExpectedOutput },
};

ValidateBatchDocumentsResult(results, expectedOutput, includeStatistics: true);

// Assert the options classes since overloads were added and the original now instantiates a RecognizeLinkedEntitiesOptions.
Assert.IsTrue(options.IncludeStatistics);
Assert.IsNull(options.ModelVersion);
Assert.AreEqual(StringIndexType.Utf16CodeUnit, options.StringIndexType);
}

[Test]
Expand Down
Loading