Skip to content

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

Merged
daspecster merged 1 commit intogoogleapis:masterfrom
daspecster:refactor-vision-requests
Nov 11, 2016
Merged

Refactor _detect_annotation() to support all annotation types.#2712
daspecster merged 1 commit intogoogleapis:masterfrom
daspecster:refactor-vision-requests

Conversation

@daspecster
Copy link
Contributor

@daspecster daspecster commented Nov 9, 2016

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 Outdated
: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
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 Outdated
"""Convert a JSON result to an entity type based on the feature."""

reverse_types = {
'FACE_DETECTION': 'faceAnnotations',

This comment was marked as spam.

Sign in to view

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

@staticmethod

This comment was marked as spam.

Sign in to view

vision/google/cloud/vision/image.py Outdated
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 Outdated
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 Outdated
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 Outdated

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 Outdated
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

daspecster commented Nov 11, 2016

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

daspecster commented Nov 11, 2016

@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

daspecster commented Nov 11, 2016

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

2 more reviewers

@tseaver tseaver tseaver left review comments

@dhermes dhermes dhermes left review comments

Reviewers whose approvals may not affect merge requirements

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