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.

Sign up for GitHub

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

Jump to bottom

Send back the complete API responses. #3059

Merged
merged 21 commits into from
Feb 24, 2017
Merged

Send back the complete API responses. #3059

merged 21 commits into from
Feb 24, 2017

Conversation

lukesneeringer
Copy link
Contributor

This pull request makes it so that we no longer drop large portions of the API responses from Natural Language.

(It also changes several copyright years.)

@lukesneeringer lukesneeringer self-assigned this Feb 23, 2017
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 23, 2017
@daspecster
Copy link
Contributor

daspecster commented Feb 23, 2017 •
edited
Loading

@lukesneeringer why the copyright year changes? Those aren't new files this year (aside from the new file in the PR :D)?

@lukesneeringer
Copy link
Contributor Author

I changed the ones I edited. (Something I should be better at doing generally.)

@lukesneeringer lukesneeringer added the api: language Issues related to the Cloud Natural Language API API. label Feb 23, 2017
@dhermes
Copy link
Contributor

dhermes commented Feb 23, 2017

I changed the ones I edited. (Something I should be better at doing generally.)

I don't know what a lawyer-cat says but @jonparrott does know (sometimes)

@daspecster
Copy link
Contributor

@lukesneeringer I thought the copyright was only for when the file was created but I could be wrong. I'm no copyright lawyer.

@daspecster
Copy link
Contributor

@lukesneeringer hyphenating makes a lot more sense. 👍

dhermes
dhermes reviewed Feb 23, 2017
View reviewed changes
Copy link
Contributor

@dhermes dhermes left a comment

Choose a reason for hiding this comment

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

I don't see any unit tests for EntityResponse, SentimentResponse or SyntaxResponse. Am I missing them?

language/google/cloud/language/api_responses.py Outdated

"""Classes representing each of the types of responses returned from
the Natural Language API.
"""

This comment was marked as spam.

Sign in to view

language/google/cloud/language/api_responses.py Outdated

class EntityResponse(object):
"""A representation of a response sent back from the
``analyzeEntites`` request to the Google Natural language API.

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

language/google/cloud/language/api_responses.py Outdated
:param payload: A dictionary representing the response.
"""
return cls(
entities=[Entity.from_api_repr(i) for i in payload['entities']],

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

language/google/cloud/language/api_responses.py Outdated

class SentimentResponse(object):
"""A representation of a response to an ``analyzeSentiment`` request
to the Google Natural Language API.

This comment was marked as spam.

Sign in to view

language/google/cloud/language/api_responses.py Outdated
:param payload: A dictionary representing the response.
"""
return cls(
language=payload.get('language', None),

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

language/google/cloud/language/entity.py Outdated
@@ -84,7 +80,7 @@ class Entity(object):
def __init__(self, name, entity_type, metadata, salience, mentions):
self.name = name
self.entity_type = entity_type
self.wikipedia_url = metadata.pop('wikipedia_url', None)
self.wikipedia_url = metadata.get('wikipedia_url', None)

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

language/google/cloud/language/sentence.py
:rtype: :class:`Sentence`
:returns: The sentence parsed from the API representation.
"""
text_span = payload['text']

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

language/google/cloud/language/sentiment.py Outdated
@@ -1,4 +1,4 @@
# Copyright 2016 Google Inc.
# Copyright 2016-2017 Google Inc.

This comment was marked as spam.

Sign in to view

language/google/cloud/language/sentence.py Outdated
# The sentence may or may not have a sentiment; only attempt the
# typecast if one is present.
sentiment = None
if payload.get('sentiment', None):

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

language/google/cloud/language/syntax.py
@@ -168,53 +169,3 @@ def from_api_repr(cls, payload):
lemma = payload['lemma']
return cls(text_content, text_begin, part_of_speech,
edge_index, edge_label, lemma)


class Sentence(object):

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@theacodes
Copy link
Contributor

Copyright is for when the file was first published, not when it was last edited.

@lukesneeringer
Copy link
Contributor Author

I also failed at updating the system tests -- working on that now.

dhermes
dhermes reviewed Feb 24, 2017
View reviewed changes
language/unit_tests/test_sentence.py Outdated
sentence = klass.from_api_repr(payload)
self.assertEqual(sentence.content, content)
self.assertEqual(sentence.begin, begin)
self.assertEqual(sentence.sentiment, None)

This comment was marked as spam.

Sign in to view

language/google/cloud/language/entity.py Outdated
@@ -84,7 +80,7 @@ class Entity(object):
def __init__(self, name, entity_type, metadata, salience, mentions):
self.name = name
self.entity_type = entity_type
self.wikipedia_url = metadata.pop('wikipedia_url', None)
self.wikipedia_url = metadata.get('wikipedia_url')

This comment was marked as spam.

Sign in to view

language/google/cloud/language/api_responses.py Outdated

class EntityResponse(object):
"""A representation of a response sent back from the
``analyzeEntites`` request to the Google Natural language API.

This comment was marked as spam.

Sign in to view

language/google/cloud/language/document.py
@@ -1,4 +1,4 @@
# Copyright 2016 Google Inc.
# Copyright 2016-2017 Google Inc.

This comment was marked as spam.

Sign in to view

language/google/cloud/language/entity.py
@@ -1,4 +1,4 @@
# Copyright 2016 Google Inc.
# Copyright 2016-2017 Google Inc.

This comment was marked as spam.

Sign in to view

language/google/cloud/language/sentence.py
# The sentence may or may not have a sentiment; only attempt the
# typecast if one is present.
sentiment = None
if payload.get('sentiment') is not None:

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that they're okay with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

@googlebot googlebot added cla: no This human has *not* signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Feb 24, 2017
@lukesneeringer lukesneeringer added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Feb 24, 2017
@lukesneeringer
Copy link
Contributor Author

@googlebot Why is that a problem, exactly? 😕 Just saying...

@dhermes dhermes merged commit bba3c86 into googleapis:master Feb 24, 2017
@lukesneeringer lukesneeringer deleted the language-api-full branch February 24, 2017 18:54
@daspecster daspecster mentioned this pull request Feb 24, 2017
Merged
19 tasks
richkadel pushed a commit to richkadel/google-cloud-python that referenced this pull request May 6, 2017
@lukesneeringer @dhermes
Send back the complete API responses. (googleapis#3059)
52e1bd4 
* Return full API responses for Natural Language.

* Return full API responses from NL.

* Do the hyphenated copyright thing for edited files.

* Update usage doc.

* Updates based on @dhermes feedback.

* Update system tests.

* Added system tests and Entity Response tests.

Still need explicit SyntaxResponse and SentimentResponse tests.

* Remove explcit dict.get('foo', None)

* Fix some of the pylint errors.

* Finish fixing pylint errors.

* It is 2017.

* Add SentimentResponse tests.

* Unit tests for SyntaxResponse.

* Missed a dict.get('foo', None) case.

* Use assertIsNone

* Remove wikipedia_url as an attribute.

* PEP 257 compliance.

* Add Sentiment isInstance check.

* Add API responses documentation.

* Adding sentences to docs.

* Fix typo.
parthea pushed a commit that referenced this pull request Jul 6, 2023
@lukesneeringer @dhermes
Send back the complete API responses. (#3059)
77a5d94 
* Return full API responses for Natural Language.

* Return full API responses from NL.

* Do the hyphenated copyright thing for edited files.

* Update usage doc.

* Updates based on @dhermes feedback.

* Update system tests.

* Added system tests and Entity Response tests.

Still need explicit SyntaxResponse and SentimentResponse tests.

* Remove explcit dict.get('foo', None)

* Fix some of the pylint errors.

* Finish fixing pylint errors.

* It is 2017.

* Add SentimentResponse tests.

* Unit tests for SyntaxResponse.

* Missed a dict.get('foo', None) case.

* Use assertIsNone

* Remove wikipedia_url as an attribute.

* PEP 257 compliance.

* Add Sentiment isInstance check.

* Add API responses documentation.

* Adding sentences to docs.

* Fix typo.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dhermes dhermes dhermes left review comments

@tseaver tseaver tseaver left review comments

Assignees

@lukesneeringer lukesneeringer

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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@lukesneeringer @daspecster @dhermes @theacodes @googlebot @tseaver