microsoft / cognitive-textanalytics-dotnet Goto Github PK

I think this abstraction is pretty broken. The Documents list here should be made protected and each derived request should enforce the correct model that it's expecting.

For example, in the SentimentRequest. The constructor should take IEnumerable and add those to the Documents property. The problem occurs when you supply an "incorrect" Document type for that request like so:

                var request = new SentimentRequest();
                request.Documents.Add(new KeyPhraseDocument
                {
                    Id = Guid.NewGuid().ToString(),
                    Text = "foo",
                });

This is compiling but the request is performing a very dangerous cast here and obviously throwing a null reference exception.

The above fix will solve that. Alternatively, the property can be moved to each derived request but the validation logic will be duplicated.

If supplying the wrong endpoint or the client uses the fallback incorrectly (i.e. when you don't have a text analytics in west us) see #5 . The rest call will get a 401. This should be thrown as some reasonable exception as every subsequent request will fail (wrong region/key).

It's not clear when you receive an empty array of results as to why it failed.

Use environment variable instead, will be useful when CI/CD process is created.

Hey there, nice job on the wrapper! Any chance of extra *Client constructors that take a URL endpoint parameter as well as the API key please? Currently we're just limited to West US. Thanks!

(I realise you can still just set the Url property of the *Client after construction, but a single constructor call would be cleaner/nice to have.)

It would be very helpful to have a client factory or a single combined client to work with. I don't see a reason to have the following:

                var client = new SentimentClient("")
                {
                    Url = "https://westeurope.api.cognitive.microsoft.com/text/analytics/v2.0/sentiment"
                };

                var client = new KeyPhraseClient("")
                {
                    Url = "https://westeurope.api.cognitive.microsoft.com/text/analytics/v2.0/keyPhrases"
                };

When something like the following would be much easier:

    public class ClientFactory
    {
        private readonly string _key;
        private readonly string _region;

        public ClientFactory(string key, string region)
        {
            _key = key;
            _region = $"https://{region}.api.cognitive.microsoft.com/text/analytics/v2.0/";
        }

        public SentimentClient CreateSentimentClient() => new SentimentClient(_key)
        {
            Url = _region + "sentiment"
        };

        public KeyPhraseClient CreateKeyPhraseClient() => new KeyPhraseClient(_key)
        {
            Url = _region + "keyPhrases"
        };
    }

The KeyPhrase request doesn't have the same checking for nulls that the sentiment request does.

Which means that it just throws unnecessarily when no language is supplied.

Last week I realized that Sentiment analysis stoped to work properly for portuguese language.

It always respond with 50%, I attach a positive and a negative cases below as evidence.

Someone can fix this, please?

I've did a test using the following code and the response has 0 documents and of course, don't have the sentiment.

using System;
using Microsoft.ProjectOxford.Text.Core;
using Microsoft.ProjectOxford.Text.Sentiment;
using Microsoft.ProjectOxford.Text;

namespace ExemploAnaliseDeTexto
{
    class Program
    {
        static void Main(string[] args)
        {
            SentimentDocument document = new SentimentDocument
            {
                Id = "1",
                Language = "en",
                Text = "I don't like when examples don't work."
            };

            SentimentRequest request = new SentimentRequest();
            request.Documents.Add(document);

            SentimentClient client = new SentimentClient("{my-api-key-here}");
            var response = client.GetSentiment(request);

            Console.Read();
        }
    }
}