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

Expect vision response data to have missing keys. #2761

Merged
merged 1 commit into from
Nov 22, 2016

Conversation

daspecster
Copy link
Contributor

Fixes #2757

I didn't add tests for the other classes, but I can if needed. Honestly, I'm not sure if and when to count on the data being there so my strategy was to try and default to None.

@daspecster daspecster added the api: vision Issues related to the Cloud Vision API. label Nov 21, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Nov 21, 2016
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.

The majority of this PR seems unneeded? You are trying to prevent key errors that will never happen / have never been reported as happening.

Let's fix the actually reported bug and then discuss the rest after?

@@ -36,8 +36,9 @@ def from_api_repr(cls, response):
:rtype: :class:`~google.cloud.vision.color.ImagePropertiesAnnotation`.
:returns: Populated instance of ``ImagePropertiesAnnotation``.
"""
colors = [ColorInformation.from_api_repr(color) for color in
response['dominantColors']['colors']]
raw_colors = response.get('dominantColors', {}).get('colors', [])

This comment was marked as spam.

@@ -61,12 +61,12 @@ def from_api_repr(cls, response):
:returns: Instance of ``EntityAnnotation``.
"""
bounds = Bounds.from_api_repr(response.get('boundingPoly'))
description = response['description']
locale = response.get('locale', None)
description = response.get('description')

This comment was marked as spam.

@@ -91,11 +91,13 @@ def from_api_repr(cls, response):
:rtype: :class:`~google.cloud.vision.face.Emotions`
:returns: Populated instance of `Emotions`.
"""
joy_likelihood = getattr(Likelihood, response['joyLikelihood'])
sorrow_likelihood = getattr(Likelihood, response['sorrowLikelihood'])
joy_likelihood = getattr(Likelihood, response.get('joyLikelihood'))

This comment was marked as spam.

class TestColorInformation(unittest.TestCase):
@staticmethod
def _get_target_class():
from google.cloud.vision.color import Color

This comment was marked as spam.

@daspecster
Copy link
Contributor Author

OK, yeah I have trust issues after learning more about how protubuf treats 0 and null values.

But yeah, later time.

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.

Deal with the nit if you please. (Please squash before merging.)

return Color

def test_rgb_color_data(self):
COLORS = {

This comment was marked as spam.

This comment was marked as spam.

@daspecster daspecster force-pushed the vision-handle-empty-values branch from 8d79e62 to fe85e0a Compare November 21, 2016 21:56
@daspecster daspecster force-pushed the vision-handle-empty-values branch from fe85e0a to ddc30eb Compare November 21, 2016 21:57
@daspecster daspecster merged commit 50d3dd1 into googleapis:master Nov 22, 2016
@daspecster daspecster deleted the vision-handle-empty-values branch November 22, 2016 02:18
richkadel pushed a commit to richkadel/google-cloud-python that referenced this pull request May 6, 2017
…y-values

Expect vision response data to have missing keys.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: vision Issues related to the Cloud Vision API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants