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.

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 sample for Natural Language 1.1 launch #352

Merged
merged 5 commits into from
Apr 21, 2017
Merged

Add sample for Natural Language 1.1 launch #352

merged 5 commits into from
Apr 21, 2017

Conversation

ace-n
Copy link
Contributor

@ace-n ace-n commented Apr 14, 2017

No description provided.

@ace-n ace-n requested review from jmdobry and gguuss April 14, 2017 09:56
Copy link
Member

@jmdobry jmdobry left a comment

Choose a reason for hiding this comment

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

Let's add NL 1.0 changes to this PR

@ace-n
Copy link
Contributor Author

ace-n commented Apr 14, 2017

Also - per @gguuss, these changes may not be in production for everyone yet. Holding off on them for now.

@ace-n ace-n added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Apr 14, 2017
Copy link
Contributor

@gguuss gguuss left a comment

Choose a reason for hiding this comment

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

This is suspect, any reason the 0th result is not getting returned?

@@ -32,7 +32,7 @@ function analyzeSentimentOfText (text) {
// Detects the sentiment of the document
document.detectSentiment()
.then((results) => {
const sentiment = results[0];
const sentiment = results[1].documentSentiment;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it just that the first result is missing sentiment? I'm seeing that not all the entities returned have sentiment.

Copy link
Contributor

@gguuss gguuss left a comment

Choose a reason for hiding this comment

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

LGTM, also IMO it's ok to publish if this doesn't affect the v1 sample.

@ace-n ace-n requested a review from sgreenberg April 21, 2017 21:06
@jmdobry jmdobry removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Apr 21, 2017
@jmdobry jmdobry merged commit 9c0d258 into master Apr 21, 2017
@jmdobry jmdobry deleted the nl-1-1 branch April 21, 2017 22:10
const entities = results[0].entities;

console.log(`Entities and sentiments:`)
entities.forEach((entity) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't forget the mentions in entity sentiment:

from Python:

    for entity in result.entities:
        print('Mentions: ')
        print(u'Name: "{}"'.format(entity.name))
        for mention in entity.mentions:
            print(u'  Begin Offset : {}'.format(mention.text.begin_offset))
            print(u'  Content : {}'.format(mention.text.content))
            print(u'  Magnitude : {}'.format(mention.sentiment.magnitude))
            print(u'  Sentiment : {}'.format(mention.sentiment.score))
            print(u'  Type : {}'.format(mention.type))
        print(u'Salience: {}'.format(entity.salience))
        print(u'Sentiment: {}\n'.format(entity.sentiment))

grayside pushed a commit that referenced this pull request Oct 26, 2022
grayside pushed a commit that referenced this pull request Nov 3, 2022
NimJay pushed a commit that referenced this pull request Nov 10, 2022
* Fix NL tests

* Add analyzeEntitySentiment sample

* Fix failing tests

* Add NL 1.0 features

* Update dependencies
NimJay pushed a commit that referenced this pull request Nov 10, 2022
* Fix NL tests

* Add analyzeEntitySentiment sample

* Fix failing tests

* Add NL 1.0 features

* Update dependencies
Shabirmean pushed a commit that referenced this pull request Feb 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants