Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Vision API features update#1353

Closed
beccasaurus wants to merge 2 commits into
masterGoogleCloudPlatform/python-docs-samples:masterfrom
vision-api-features-updateGoogleCloudPlatform/python-docs-samples:vision-api-features-updateCopy head branch name to clipboard
Closed

Vision API features update#1353
beccasaurus wants to merge 2 commits into
masterGoogleCloudPlatform/python-docs-samples:masterfrom
vision-api-features-updateGoogleCloudPlatform/python-docs-samples:vision-api-features-updateCopy head branch name to clipboard

Conversation

@beccasaurus

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 googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 7, 2018
@beccasaurus beccasaurus requested a review from dizcology February 7, 2018 21:03
@beccasaurus

beccasaurus commented Feb 8, 2018

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

@theacodes

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

beccasaurus commented Feb 8, 2018 via email

Copy link
Copy Markdown
Contributor Author

@theacodes

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

beccasaurus commented Feb 8, 2018 via email

Copy link
Copy Markdown
Contributor Author

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

block_text = ''

@beccasaurus beccasaurus Feb 13, 2018

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.

@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 beccasaurus Feb 13, 2018

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.

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 beccasaurus Feb 13, 2018

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.

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.

ping @dizcology for thoughts

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

@beccasaurus

Copy link
Copy Markdown
Contributor Author

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

@beccasaurus

Copy link
Copy Markdown
Contributor Author

Closing PR /cc @dizcology

@beccasaurus beccasaurus closed this Aug 3, 2018
@beccasaurus 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
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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.