-
Notifications
You must be signed in to change notification settings - Fork 6.4k
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.
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
Speech GCS #784
Speech GCS #784
Conversation
speech/cloud-client/transcribe.py
Outdated
@@ -57,13 +55,48 @@ def main(speech_file): | |||
# [END send_request] | |||
|
|||
|
|||
def transcribe_gcs(gcs_uri): | |||
"""Transcribes the audio file specified by the gcs_uri.""" | |||
# [START authenticating_gcs] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Meta: seems unnecessary to chop this sample up into so many regions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, these aren't even referenced anywhere in the docs yet, I'd just stuck to the inherited convention.
speech/cloud-client/transcribe.py
Outdated
|
||
# [START construct_request_gcs] | ||
audio_sample = speech_client.sample( | ||
None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is None
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
content as in there's either content or a gcs_uri
speech/cloud-client/transcribe.py
Outdated
# [END send_request_gcs] | ||
|
||
|
||
def main(path): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We generally don't put main functions in our samples, just place this logic below or do separate subparsers for local path and gcs path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, moved logic to right after the args are parsed
operation.poll() | ||
|
||
if not operation.complete: | ||
print("Operation not complete and retry limit reached.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
single quotes everywhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, I should add a module to my pavlok for such transgressions.
while retry_count > 0 and not operation.complete: | ||
retry_count -= 1 | ||
time.sleep(2) | ||
operation.poll() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought they were going to change this interface soon.... hmm. @dhermes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_OperationFuture
has landed in GAX/GAPIC interfaces and we are slowly working it in
/cc @lukesneeringer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, cool, we'll catch it when @dpebot bumps our dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll skip this for now.
|
||
|
||
def test_transcribe_gcs(resource, capsys): | ||
main('gs://cloud-samples-tests/speech/brooklyn.flac') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer these to be in the python-docs-samples-tests bucket, provided by cloud_config.bucket
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
@@ -13,11 +13,18 @@ | |||
|
|||
import re | |||
|
|||
from transcribe_async import main | |||
from transcribe_async import transcribe_file, transcribe_gcs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Final nit: import modules, not names. Just do import transcribe_async
and use transcribe_async.transcribe_file
, etc.
Adds examples showing transcription from a GCS URI.