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

Refactor _detect_annotation() to support all annotation types. #2712

Merged
merged 1 commit into from
Nov 11, 2016
Merged

Refactor _detect_annotation() to support all annotation types. #2712

merged 1 commit into from
Nov 11, 2016

Conversation

daspecster
Copy link
Contributor

This is a step towards adding the manual detect method.

See: #2697

@daspecster daspecster added the api: vision Issues related to the Cloud Vision API. label Nov 9, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Nov 9, 2016
@daspecster daspecster force-pushed the refactor-vision-requests branch from 51433d6 to 7d4a998 Compare November 9, 2016 21:08
@daspecster daspecster changed the title Refactor so _detect_annotation() support all types. Refactor _detect_annotation() to support all annotation types. Nov 9, 2016
@daspecster daspecster force-pushed the refactor-vision-requests branch from 7d4a998 to e766ac9 Compare November 10, 2016 18:05
dhermes
dhermes reviewed Nov 10, 2016
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.

Mostly LGTM though ISTM you should be doing

features = [feature]

in _detect_annotation

vision/google/cloud/vision/image.py
:type feature: :class:`~google.cloud.vision.feature.Feature`
:param feature: The ``Feature`` indication the type of annotation to
perform.
:type features: list of:class:`~google.cloud.vision.feature.Feature`

This comment was marked as spam.

Sign in to view

vision/google/cloud/vision/image.py
@@ -85,28 +85,58 @@ def source(self):
"""
return self._source

def _detect_annotation(self, feature):
def _detect_annotation(self, features):

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

vision/google/cloud/vision/image.py
reverse_types = {
'FACE_DETECTION': 'faceAnnotations',

This comment was marked as spam.

Sign in to view

vision/google/cloud/vision/image.py
return detected_objects[0]
return detected_objects

@staticmethod

This comment was marked as spam.

Sign in to view

vision/google/cloud/vision/image.py
self._entity_from_response_type(feature.feature_type, results))

if len(detected_objects) == 1:
return detected_objects[0]

This comment was marked as spam.

Sign in to view

vision/google/cloud/vision/image.py
result = results[feature_key]
detected_objects.append(SafeSearchAnnotation.from_api_repr(result))
else:
for result in results[feature_key]:

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

vision/google/cloud/vision/image.py
detected_objects.append(detected_object)
feature_key = reverse_types[feature_type]

if feature_type == 'FACE_DETECTION':

This comment was marked as spam.

Sign in to view

vision/google/cloud/vision/image.py

if feature_type == 'FACE_DETECTION':
for face in results[feature_key]:
detected_objects.append(Face.from_api_repr(face))

This comment was marked as spam.

Sign in to view

vision/google/cloud/vision/image.py
detected_objects.append(SafeSearchAnnotation.from_api_repr(result))
else:
for result in results[feature_key]:
detected_objects.append(EntityAnnotation.from_api_repr(result))

This comment was marked as spam.

Sign in to view

@daspecster
Copy link
Contributor Author

Regarding your comment about features = [features], if I do that, then I'll need to put a check there as well.

if not isinstance(features, list):
    features = [features]

tseaver
tseaver reviewed Nov 11, 2016
View reviewed changes
vision/google/cloud/vision/image.py
'LOGO_DETECTION': 'logoAnnotations',
_SAFE_SEARCH_DETECTION: 'safeSearchAnnotation',
'TEXT_DETECTION': 'textAnnotations',
}

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@dhermes
Copy link
Contributor

dhermes commented Nov 11, 2016

if I do that, then I'll need to put a check there as well.

No, just make your callers obey the signature. So if it accepts one feature (which all the current callers so) then they send one feature.

However, as I mentioned before, just leave the change in.

@daspecster
Copy link
Contributor Author

@dhermes this is lacking clarity as you mentioned before because I'm preparing for #2697.
detect() will need to pass a list of features. Which will be clear in the next PR for this.

@daspecster daspecster force-pushed the refactor-vision-requests branch from 1bfd25f to f164c3f Compare November 11, 2016 19:26
@daspecster
Copy link
Contributor Author

Squished. Once the build goes green I'll merge.

@daspecster daspecster force-pushed the refactor-vision-requests branch from f164c3f to 83b128a Compare November 11, 2016 19:27
@daspecster daspecster merged commit 0291dd4 into googleapis:master Nov 11, 2016
@daspecster daspecster deleted the refactor-vision-requests branch November 11, 2016 19:50
@daspecster daspecster mentioned this pull request Nov 17, 2016
Merged
richkadel pushed a commit to richkadel/google-cloud-python that referenced this pull request May 6, 2017
@daspecster
Merge pull request googleapis#2712 from daspecster/refactor-vision-re…
9e27531 
…quests

Refactor _detect_annotation() to support all annotation types.
parthea pushed a commit that referenced this pull request Oct 21, 2023
@daspecster
Merge pull request #2712 from daspecster/refactor-vision-requests
72733cc 
Refactor _detect_annotation() to support all annotation types.
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

@tseaver tseaver

Labels
api: vision Issues related to the Cloud Vision 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.
4 participants
@daspecster @dhermes @tseaver @googlebot