language: analyzeEntitySentiment support

[callmehiphop]

This adds support for the analyzeEntitySentiment rpc.

[stephenplusplus]

Is this going to replace detectSentiment?

[callmehiphop]

Is this going to replace detectSentiment?

Not sure, but analyzeSentiment and analyzeEntities are still both available in the v1beta2 proto files, so I would assume not.

[coveralls]

Coverage remained the same at 100.0% when pulling 85270d7 on callmehiphop:dg--nl-entity-sentiment into 20cc892 on GoogleCloudPlatform:master.

[stephenplusplus]

Docs!

[stephenplusplus]

• detectEntitySentiment = detect entities; include their sentiment scores
• detectEntities = detect entities; don't include their sentiment scores

Would things be simpler if we could just use an option to distinguish between intent, as opposed to multiple methods?

language.detectEntities({ sentiment: false }) // default
language.detectEntities({ sentiment: true }) // calls detectEntitySentiment

Better yet, how about we just always include sentiment? Is there something expensive about this method that it needs to be separated from the one that doesn't include sentiment? The API response is exactly the same, with the exception of the sentiment object being present or not.

[callmehiphop]

Better yet, how about we just always include sentiment? Is there something expensive about this method that it needs to be separated from the one that doesn't include sentiment? The API response is exactly the same, with the exception of the sentiment object being present or not.

@stephenplusplus I think it would be better to get some feedback from googlers on this.

/cc @monattar @landrito @lukesneeringer

[coveralls]

Coverage remained the same at 100.0% when pulling ef93ee7 on callmehiphop:dg--nl-entity-sentiment into 20cc892 on GoogleCloudPlatform:master.

[monattar]

Wrt question about why not always including sentiment: This feature is more expensive than all other features. We shouldn't include the sentiment by default, as it will incur high costs that the user might not intend.

We thought quite a bit about where to expose this API (within analyzeEntity, as a separate method, etc), and separate method has been the best. Please do not change the surface.

[stephenplusplus]

Thanks for the information.

and separate method has been the best.

Why?

[lukesneeringer]

We do not want to merge this for a couple of reasons.

• We do not want to opt people in to the 1.1 beta API. Users should get v1 unless they explicitly request otherwise.
• We have decided to only expose the new method on the GAPIC for now (and DPEs are handling documenting this).

Sorry, @callmehiphop -- I did not realize you were working on this last week, or I would have updated you sooner.

This PR is superceded by #2203, which is done the way we (Google internal) settled on yesterday.