[TA] Added MultiCategoryClassify Functionality #24237
Conversation
AhmedLeithy
requested review from
annelo-msft,
KrzysztofCwalina,
maririos and
tg-msft
as code owners
ghost
added
the
DocumentClassificationCollection --> ClassificationCategoryCollection
AhmedLeithy
force-pushed
the
TA-MultiCategoryClassify
branch
from
cb2ad35 to
fc0f362
Compare
| }; | ||
|
|
||
| // Set project and deployment names of the target model | ||
| #if SNIPPET |
There was a problem hiding this comment.
looks like there is no snippet defined in this class, so this section of code is not necessary. you could just do
string projectName = TestEnvironment.MultiClassificationProjectName;
string deploymentName = TestEnvironment.MultiClassificationDeploymentName;
There was a problem hiding this comment.
Same for the other 3 sample classes
There was a problem hiding this comment.
Sure thing, removed them.
I had them there because I remember in DT there were some samples where we wanted to have the #if SNIPPET directives for clarity, even when the code was not actually used in a snippet.
There was a problem hiding this comment.
Fair. DT was more complex as it required function calls to get the storage setup. here it is just a string so user can follow easier
| { | ||
| foreach (MultiCategoryClassifyResult documentResults in classificationActionResults.DocumentsResults) | ||
| { | ||
| if (documentResults.ClassificationCategories.Count > 0) |
There was a problem hiding this comment.
I thought arch board feedback was to rename this property to classifications? @mssfang
There was a problem hiding this comment.
Thanks for pointing that out. I seem to have missed this rename in SingleCategoryClassifyResult as well.
SingleCategoryClassifyResult.ClassificationCategory -> SingleCategoryClassifyResult.Classification
MultiCategoryClassifyResult.ClassificationCategories -> MultiCategoryClassifyResult.Classifications
| new MultiCategoryClassifyAction(FakeProjectName, FakeDeploymentName), | ||
| new MultiCategoryClassifyAction(FakeProjectName, FakeDeploymentName) | ||
| { | ||
| DisableServiceLogs = true | ||
| } | ||
| }, | ||
| }; | ||
|
|
||
| ArgumentException ex = Assert.ThrowsAsync<ArgumentException>(async () => await client.StartAnalyzeActionsAsync(documents, batchActions)); | ||
| Assert.AreEqual("Multiple of the same action is not currently supported.", ex.Message); |
There was a problem hiding this comment.
I believe this test should work if different values for project name / deployment name are passed for each action
There was a problem hiding this comment.
not following the logic we currently have in the .NET SDK.
I prefer to leave this as is so we can make progress in this PR, work on the custom entities PR, and once those 2 are merged, then focus on the work on #24430
Rename property ClassificationCategory -> Classification
* [TextAnalytics] Generated client from 3.2-preview.2 swagger (#23536) * [TA] Added SingleCategoryClassify functionality (#24235) * [TA] Added MultiCategoryClassify Functionality (#24237) * [TA] Added RecognizeCustomEntities Functionality (#24245) * [TA] Expose ActionName and enable multiple actions from same type (#24619) * Rerecorded all tests excluding AAD ones (#24913) * re-record AAD tests (#24919) * [TA] Enable CI for live tests for custom features (#24916) * add comments Co-authored-by: Caio Saldanha <camaiaor@microsoft.com> Co-authored-by: Ahmed Leithy <v-aleithy@microsoft.com> Co-authored-by: Salah Mostafa <zulamostafa@gmail.com> Co-authored-by: Salah Mostafa <v-samostafa@microsoft.com>
This PR contains the implementation of the Multi Category Classify API interface as explained in #24062. Renames post the meeting with the architecture board have been taken into consideration in this PR.
The CI pipeline currently fails because:
The tests can be rerecorded in another PR at a later date.