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

Vision API features update by beccasaurus · Pull Request #1353 · GoogleCloudPlatform/python-docs-samples · GitHub

Vision API features update - #1353

Closed
beccasaurus wants to merge 2 commits into
masterfrom
vision-api-features-update
Closed

Vision API features update#1353
beccasaurus wants to merge 2 commits into
masterfrom
vision-api-features-update

Conversation

Copy link
Copy Markdown
Contributor

This is a REDO of the previous PR: Vision API features update #1339

(That PR deleted some Beta samples which are still required by the docs)

Reverted & brought back the Beta samples!

Let's try this again 😄

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 7, 2018
beccasaurus requested a review from dizcology February 7, 2018 21:03

beccasaurus commented Feb 8, 2018
edited
Loading

Copy link
Copy Markdown
Contributor Author

I noticed that detect.py does not use the canonical region tags for its snippets (there are lots of unconventional def_.... region tags)

Is this a good time to change those? I was trying to copy detect.py, treating it as if it's the canonical, but it's really NOT CANONICAL for Vision snippets

/cc @sirtorry

Copy link
Copy Markdown
Contributor

@remi totally up to you. Python uses indented_block because it keeps things simple and generally we can name the functions the same as the region tags (go also has similar go_func functionality). But it doesn't matter either way style-wise, it's just preference.

beccasaurus commented Feb 8, 2018 via email

Copy link
Copy Markdown
Contributor Author

Copy link
Copy Markdown
Contributor

Yep - hence my last comment- totally up to you. Within this repo there are samples that use both styles in the same file, even.

beccasaurus commented Feb 8, 2018 via email

Copy link
Copy Markdown
Contributor Author

print(u'Paragraph Confidence: {}\n'.format(
paragraph.confidence))

block_text = ''

beccasaurus Feb 13, 2018
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@dizcology I really, really struggle to easily understand what this sample is doing

It's not easy on the eyes & doesn't read like prose on the website IMO

Do you like this sample? Any ideas for ways to highlight parts TextAnnotation that developers want to know about in a more consumable, easy to read sample?

beccasaurus Feb 13, 2018
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

For Ruby, I'm rolling with this:

  # image_path = "Google Cloud Storage URI, eg. 'gs://my-bucket/image.png'"

  require "google/cloud/vision"

  vision = Google::Cloud::Vision.new

  document = vision.document_text_detection(image_path).full_text_annotation

  puts "Full document text: #{document.text}"

  document.pages.each do |page|
    page.blocks.each  do |block|
      block.paragraphs.each do |paragraph|
        puts "Paragraph confidence: #{paragraph.confidence}"
        paragraph.words.each do |word|
          puts "\tWord confidence: #{word.confidence}"
          puts "\tWord text: #{word.symbols.map(&:text).join}"
          word.symbols.each do |symbol|
            puts "\t\tSymbol text: #{symbol.text}"
            puts "\t\tSymbol confidence: #{symbol.confidence}"
          end
        end
      end
    end
  end

I almost don't want to include anything that concatenates/joins text together to print it out (developers can figure that out?) ... but, at least in Ruby it's idiomatic and relatively readable to print out each word:

puts "\tWord text: #{word.symbols.map(&:text).join}"

The Python snippet includes:

  • 2 Lists used to build up data to print
  • 2 Strings used to build data to print
  • Logic to extend Lists
  • Also, 2 types of syntax are used to concatenate strings:
word_text = word_text + symbol.text
block_text += ' ' + word_text

^--- both should use += or not use += (should be consistent)

Thoughts?

beccasaurus Feb 13, 2018
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Also: I vote that the sample shows how to print out the WHOLE text without having to concatenate symbols⇒words⇒paragraphs⇒blocks⇒pages, because this is a very common use-case

^---- also, what is a block? ... I hope the docs make that clear, because it's not obvious to me ...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

ping @dizcology for thoughts

Also, would love to get this ready for merge ~ this comment is my only remaining question IIRC 😄

Copy link
Copy Markdown
Contributor Author

Hey, @dizcology we can close this, right? Vision GA was merged #1427

Copy link
Copy Markdown
Contributor Author

Closing PR /cc @dizcology

beccasaurus closed this Aug 3, 2018
beccasaurus deleted the vision-api-features-update branch August 8, 2018 21:17
chalmerlowe pushed a commit that referenced this pull request Apr 7, 2026
#1353)

* test: test universe domain client only in prod

* unflake hmac snippet test
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants


Back | FazBrowse Home | New Git URL