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

Language updates #1783

Merged
merged 9 commits into from
Nov 15, 2016
Merged

Language updates #1783

merged 9 commits into from
Nov 15, 2016

Conversation

callmehiphop
Copy link
Contributor

@callmehiphop callmehiphop commented Nov 10, 2016

Updates Natural Language to 1.0 proto files!

⚠️ BLOCKED

Notable Changes

  • New rcp AnalyzeSyntax in the form of document#detectSyntax
  • Sentences can now contain sentiments
  • sentiment.polarity was renamed to sentiment.score
  • partOfSpeech received 11 new attributes
  • English, Japanese and Spanish now supported for all features.
  • There are additional changes, but generally only affect verbose mode.

Possible Conflicts (per API team review)

  • Multiplying values by 100 appears to be problematic. While score (polarity) returns a value between -1 and 1, magnitude is unbounded. That being said I'm not sure we're creating a meaningful number for magnitude by multiplying it.

TODO

  • Unit tests

@callmehiphop callmehiphop added don't merge api: language Issues related to the Cloud Natural Language API API. labels Nov 10, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Nov 10, 2016
@callmehiphop
Copy link
Contributor Author

@stephenplusplus PTAL!

@stephenplusplus
Copy link
Contributor

+1 on not multiplying magnitude.

@callmehiphop
Copy link
Contributor Author

+1 on not multiplying magnitude.

Awesome, that being said do you think we should be consistent and not multiply score as well?

@stephenplusplus
Copy link
Contributor

stephenplusplus commented Nov 10, 2016

No, they're not the same value range, so we can't achieve consistency between them. All of our APIs multiply -1.0 through 1.0 ranges.

@@ -196,13 +197,34 @@ Document.PART_OF_SPEECH = {
* text.
* @param {object[]} callback.annotation.tokens - Parts of speech that were
* detected from the text.
* @param {object[]} callback.annotation.tokens.text - The piece of text
* @param {string} callback.annotation.tokens[].text - The piece of text

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

* // 'Google is an American multinational technology company' +
* // 'specializing in Internet-related services and products.'
* // beginOffset: -1
* // text: {

This comment was marked as spam.

This comment was marked as spam.

* @param {boolean} options.verbose - Enable verbose mode for more detailed
* results. Default: `false`
* @param {function} callback - The callback function.
* @param {?error} callback.err - An error occurred while making this request.
* @param {number} callback.sentiment - A value in the range of `-100` to `100`.
* Large numbers represent more positive sentiments.
* @param {number} callback.sentiment.sentiment - A value in the range of `-100`

This comment was marked as spam.

This comment was marked as spam.

* // content:
* // 'Google is an American multinational technology company' +
* // 'specializing in Internet-related services and products.',
* // beingOffset: -1

This comment was marked as spam.

This comment was marked as spam.

@@ -331,7 +359,18 @@ describe('Language', function() {
assert.deepEqual(annotation.tokens[0], {
text: 'Hello',
partOfSpeech: 'Other: foreign words, typos, abbreviations',
partOfSpeechTag: 'X'
tag: 'X',
aspect: 'ASPECT_UNKNOWN',

This comment was marked as spam.

This comment was marked as spam.

@jgeewax
Copy link
Contributor

jgeewax commented Nov 11, 2016

Are we going to change the version to 1.0? Or is this meaning something else? Not sure if we're ready to say 1.0 here....

@callmehiphop
Copy link
Contributor Author

@jgeewax sorry about the confusion, we're moving from the upstream v1beta1 api to the upstream v1 api. We will not be publishing the library itself as 1.0

* @param {boolean} options.verbose - Enable verbose mode for more detailed
* results. Default: `false`
* @param {function} callback - The callback function.
* @param {?error} callback.err - An error occurred while making this request.
* @param {number} callback.sentiment - A value in the range of `-100` to `100`.
* Large numbers represent more positive sentiments.
* @param {number} callback.sentiment - A value in the range of `-100`

This comment was marked as spam.

This comment was marked as spam.

@@ -601,8 +660,21 @@ Document.prototype.detectEntities = function(options, callback) {
* }
*
* // sentiment = {
* // polarity: 100,
* // magnitude: 40
* // sentiment: {

This comment was marked as spam.

This comment was marked as spam.


var syntax = {
sentences: Document.formatSentences_(resp.sentences, verbose),
tokens: Document.formatTokens_(resp.tokens, verbose),

This comment was marked as spam.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 2005c08 on callmehiphop:language-updates into * on GoogleCloudPlatform:master*.

* text.
* @param {object[]} callback.syntax.tokens - Parts of speech that were
* detected from the text.
* @param {string} callback.syntax.language - The language detected from the

This comment was marked as spam.

This comment was marked as spam.

* results. Default: `false`
* @param {function} callback - The callback function.
* @param {?error} callback.err - An error occurred while making this request.
* @param {object} callback.syntax - The recognizd syntax from the text.

This comment was marked as spam.

This comment was marked as spam.

* @resource [documents.analyzeSyntax API Documentation]{@link https://cloud.google.com/natural-language/reference/rest/v1/documents/analyzeSyntax}
*
* @param {object=} options - Configuration object. See
* [documents.annotateText](https://cloud.google.com/natural-language/reference/rest/v1/documents/analyzeSyntax#request-body).

This comment was marked as spam.

This comment was marked as spam.

* // tokens: [
* // {
* // text: 'Google',
* // partOfSpeech: 'Noun (common and proper)'

This comment was marked as spam.

This comment was marked as spam.

* // },
* // dependencyEdge: {
* // headTokenIndex: 1,
* // label: 'NSUBJ'

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.


for (var part in token) {
if (token.hasOwnProperty(part) && /UNKNOWN/.test(token[part])) {
token[part] = false;

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@stephenplusplus stephenplusplus removed their assignment Nov 14, 2016
@stephenplusplus
Copy link
Contributor

Please re-assign when it's ready for another look.

@stephenplusplus stephenplusplus added status: blocked Resolving the issue is dependent on other work. and removed don't merge labels Nov 14, 2016
@stephenplusplus
Copy link
Contributor

LGTM. Re-assign when the blockers are cleared.

@stephenplusplus stephenplusplus removed their assignment Nov 14, 2016
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 62d397d on callmehiphop:language-updates into * on GoogleCloudPlatform:master*.

@jmuk
Copy link
Contributor

jmuk commented Nov 14, 2016

googleapis/googleapis#190 has been merged, and the new proto is already in the master of https://github.com/googleapis/googleapis. @stephenplusplus, please update google-proto-files package.

@bjwatson
Copy link

bjwatson commented Nov 15, 2016

@callmehiphop @jmdobry googleapis/googleapis#190 has been merged. Can one of you take care of updating google-proto-files?

@jmdobry
Copy link
Contributor

jmdobry commented Nov 15, 2016

@bjwatson
Copy link

Thanks @jmdobry and @stephenplusplus.

@jmdobry
Copy link
Contributor

jmdobry commented Nov 15, 2016

Language v1 proto is available in google-proto-files v0.8.5.

@bjwatson
Copy link

bjwatson commented Nov 15, 2016

Thanks @jmdobry.

@stephenplusplus have your requested changes been satisfied?

Also, Travis seems unhappy. Is this just flakiness?

@bjwatson
Copy link

Dave explained that the unhappiness is just depending on the google-proto-files update.

@stephenplusplus
Copy link
Contributor

Yep, just update google-proto-files to 0.9.0 (@callmehiphop simultaneously published to npm).

@stephenplusplus stephenplusplus removed the status: blocked Resolving the issue is dependent on other work. label Nov 15, 2016
@callmehiphop
Copy link
Contributor Author

Should be good to go on this.

@stephenplusplus stephenplusplus merged commit bd70843 into googleapis:master Nov 15, 2016
@jmdobry
Copy link
Contributor

jmdobry commented Nov 15, 2016

@callmehiphop Remember to get your google-proto-files 0.9.0 commit pushed to GitHub

@callmehiphop
Copy link
Contributor Author

@jmdobry I ended up unpublishing it since you beat me to it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: language Issues related to the Cloud Natural Language API API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants