FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Initial vision API addition. by daspecster · Pull Request #1803 · googleapis/google-cloud-python · GitHub

/ google-cloud-python Public

Initial vision API addition. - #1803

Closed
daspecster wants to merge 2 commits into
googleapis:masterfrom
daspecster:vision
ConversationCommits (2)ChecksFiles changed
Closed

Initial vision API addition.#1803
daspecster wants to merge 2 commits into
googleapis:masterfrom
daspecster:vision

Conversation

daspecster commented May 17, 2016

Copy link
Copy Markdown
Contributor

Fairly rough and a lot of conversation to be had.

  • Face
  • Landmark
  • Logo
  • Label
  • Text
  • Safe Search

Examples to play with

>>> from gcloud.vision import Client
>>> vc = Client('ferrous-arena')
>>> f = open('/tmp/car.jpg', 'r')
>>> vc.annotate(f.read(), 'LABEL_DETECTION', 3)
>>> vc.annotate('gs://ferrous-arena-my-test-bucket/car.jpg', 'LABEL_DETECTION', 3)

daspecster added do not merge Indicates a pull request not ready for merge, due to either quality or timing. api: vision Issues related to the Cloud Vision API. labels May 17, 2016
daspecster added this to the Vision Alpha milestone May 17, 2016
googlebot added the cla: yes This human has signed the Contributor License Agreement. label May 17, 2016
dhermes reviewed May 17, 2016
View reviewed changes
Comment thread gcloud/vision/annotation.py Outdated
_topicality = None
_bounding_poly = None
_locations = []
_properties = []

This comment was marked as spam.

Sign in to view

theacodes commented May 17, 2016

Copy link
Copy Markdown
Contributor 
vc.annotate(f.read(), 'LABEL_DETECTION', 3)

I'd prefer constants over strings, even if they semantically mean the same thing:

vc.annotate(f.read(), vision.LABEL_DETECTION, 3)

dhermes reviewed May 17, 2016
View reviewed changes
Comment thread gcloud/vision/client.py Outdated
def as_dict(self):
return {
"image": self.image,
"features": self.features

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

dhermes commented May 17, 2016

Copy link
Copy Markdown
Contributor

Rather than writing and dumping all this code, please draft a vision-usage.rst doc and we can discuss features before there is code.

Once we agree on something, try to land them feature by feature so review doesn't become unmanageable. (e.g. you couldn't just sent a tiny PR with the Connection, then a tiny PR with the Client and then build up the client for each feature).

(PS I know I "broke the rules" in #1772 however the entire API is contained in three methods so I did it "on purpose" / was aware it was a bit of a brain dump.)

tseaver commented May 17, 2016

Copy link
Copy Markdown
Contributor

(PS I know I "broke the rules" in #1772 however the entire API is contained in three methods so I did it "on purpose" / was aware it was a bit of a brain dump.)

#1772 has have 70+ comments on it, which is a sign that we should've argued out the API surface first. :)

dhermes commented May 17, 2016

Copy link
Copy Markdown
Contributor

@tseaver I totally agree and wish I had done that.

daspecster commented May 17, 2016

Copy link
Copy Markdown
Contributor Author

Ok, I'll make a PR with just a vision-usage.rst and then we can talk about it there.

daspecster mentioned this pull request May 18, 2016
Closed

tseaver commented May 23, 2016

Copy link
Copy Markdown
Contributor

In favor of #1809 ans subsequent.

tseaver closed this May 23, 2016
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

api: vision Issues related to the Cloud Vision API. cla: yes This human has signed the Contributor License Agreement. do not merge Indicates a pull request not ready for merge, due to either quality or timing.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL