-
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 gapic client library #1012
Speech gapic client library #1012
Conversation
speech/cloud-client/requirements.txt
Outdated
@@ -1 +1 @@ | |||
google-cloud-speech==0.26.0 | |||
google-cloud-speech>=0.27.0 |
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.
Prefer all requirements be ==
, for maximum reproducibility.
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
language_code='en-US') | ||
|
||
# In practice requests should be a generator yielding chunks of audio data. | ||
requests = [request] |
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.
Might be clearer if you only construct the StreamingRecognizeRequest
within a generator. ie:
requests = (types.StreamingRecognizeRequest(audio_content=c) for c in [content])
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
# [END audio_stream] | ||
|
||
|
||
def listen_print_loop(results_gen): | ||
def listen_print_loop(responses): | ||
"""Iterates through server responses and prints them. | ||
|
||
The results_gen passed is a generator that will block until a response |
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.
Should update the docstring as well
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.
updated.
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.
Looks mostly good to me, with a few nits.
speech/cloud-client/quickstart.py
Outdated
audio = types.RecognitionAudio(content=content) | ||
|
||
config = types.RecognitionConfig( | ||
encoding='LINEAR16', |
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.
Is there an enum for this?
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.
There is, but requires the additional import of from google.cloud.speech import enums
. I was keeping it consistent with the existing samples.
language_code='en-US') | ||
|
||
# In practice requests should be a generator yielding chunks of audio data. | ||
requests = (types.StreamingRecognizeRequest(audio_content=c) |
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.
please use a more descriptive variable name rather than c
.
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.
encoding='LINEAR16', | ||
sample_rate_hertz=RATE, | ||
language_code=language_code) | ||
streaming_config = types.StreamingRecognitionConfig(config=config, |
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.
nit: start a newline at the opening (
to avoid hanging indents like this.
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.
Is it not in types?
…On Thu, Jul 13, 2017, 2:25 PM Yu-Han Liu ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In speech/cloud-client/quickstart.py
<#1012 (comment)>
:
> @@ -35,14 +36,16 @@ def run_quickstart():
# Loads the audio into memory
with io.open(file_name, 'rb') as audio_file:
content = audio_file.read()
- sample = speech_client.sample(
- content,
- source_uri=None,
- encoding='LINEAR16',
- sample_rate_hertz=16000)
+ audio = types.RecognitionAudio(content=content)
+
+ config = types.RecognitionConfig(
+ encoding='LINEAR16',
There is, but requires the additional import of from google.cloud.speech
import enums. I was keeping it consistent with the existing samples.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#1012 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAPUc62HIj0vuRiwZogfRuWk-1p2ZF2_ks5sNotegaJpZM4OOqyr>
.
|
in the current form of the GAPIC client library |
I would still strongly prefer using an enum over magic values.
…On Thu, Jul 13, 2017, 2:31 PM Yu-Han Liu ***@***.***> wrote:
in the current form of the GAPIC client library enums is its own module.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#1012 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAPUc1vVDFylMQa2fm3ttba38Orwzup5ks5sNozKgaJpZM4OOqyr>
.
|
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.
This LGTM, but I can't merge it until the 0.27.0 version of the library is published. Have you run these tests locally?
I have tested with a client library provided to me to migrate these samples, but yes, let's wait after the package has been published and I have the chance to run the test against the published client. |
package published: https://pypi.python.org/pypi/google-cloud-speech/0.27.0 |
…-samples#1012) * Migrate quickstart to GAPIC client library * Migrate transcribe to GAPIC client library * Migrate transcribe_async to GAPIC client library * Migrate transcribe_streaming to GAPIC client library * clean up * clean up * Import from google.cloud.speech * update transcribe samples * import in alphabetic order * remove unused variable * use strings instead of enums * restructure code * comment on sreaming requests * import style * flake * correct indent * migrate transcribe_streaming_mic to gapic * update google-cloud-speech version requirement * addressing review comments * at the end of the audio stream, put None to signal to the generator * flake * addressing github review comments * add region tags for migration guide * update README * rst format * bullet * addressing PR review comments * use enums * remove a word
…-samples#1012) * Migrate quickstart to GAPIC client library * Migrate transcribe to GAPIC client library * Migrate transcribe_async to GAPIC client library * Migrate transcribe_streaming to GAPIC client library * clean up * clean up * Import from google.cloud.speech * update transcribe samples * import in alphabetic order * remove unused variable * use strings instead of enums * restructure code * comment on sreaming requests * import style * flake * correct indent * migrate transcribe_streaming_mic to gapic * update google-cloud-speech version requirement * addressing review comments * at the end of the audio stream, put None to signal to the generator * flake * addressing github review comments * add region tags for migration guide * update README * rst format * bullet * addressing PR review comments * use enums * remove a word
* Migrate quickstart to GAPIC client library * Migrate transcribe to GAPIC client library * Migrate transcribe_async to GAPIC client library * Migrate transcribe_streaming to GAPIC client library * clean up * clean up * Import from google.cloud.speech * update transcribe samples * import in alphabetic order * remove unused variable * use strings instead of enums * restructure code * comment on sreaming requests * import style * flake * correct indent * migrate transcribe_streaming_mic to gapic * update google-cloud-speech version requirement * addressing review comments * at the end of the audio stream, put None to signal to the generator * flake * addressing github review comments * add region tags for migration guide * update README * rst format * bullet * addressing PR review comments * use enums * remove a word
* Migrate quickstart to GAPIC client library * Migrate transcribe to GAPIC client library * Migrate transcribe_async to GAPIC client library * Migrate transcribe_streaming to GAPIC client library * clean up * clean up * Import from google.cloud.speech * update transcribe samples * import in alphabetic order * remove unused variable * use strings instead of enums * restructure code * comment on sreaming requests * import style * flake * correct indent * migrate transcribe_streaming_mic to gapic * update google-cloud-speech version requirement * addressing review comments * at the end of the audio stream, put None to signal to the generator * flake * addressing github review comments * add region tags for migration guide * update README * rst format * bullet * addressing PR review comments * use enums * remove a word
* Migrate quickstart to GAPIC client library * Migrate transcribe to GAPIC client library * Migrate transcribe_async to GAPIC client library * Migrate transcribe_streaming to GAPIC client library * clean up * clean up * Import from google.cloud.speech * update transcribe samples * import in alphabetic order * remove unused variable * use strings instead of enums * restructure code * comment on sreaming requests * import style * flake * correct indent * migrate transcribe_streaming_mic to gapic * update google-cloud-speech version requirement * addressing review comments * at the end of the audio stream, put None to signal to the generator * flake * addressing github review comments * add region tags for migration guide * update README * rst format * bullet * addressing PR review comments * use enums * remove a word
…-samples#1012) * Migrate quickstart to GAPIC client library * Migrate transcribe to GAPIC client library * Migrate transcribe_async to GAPIC client library * Migrate transcribe_streaming to GAPIC client library * clean up * clean up * Import from google.cloud.speech * update transcribe samples * import in alphabetic order * remove unused variable * use strings instead of enums * restructure code * comment on sreaming requests * import style * flake * correct indent * migrate transcribe_streaming_mic to gapic * update google-cloud-speech version requirement * addressing review comments * at the end of the audio stream, put None to signal to the generator * flake * addressing github review comments * add region tags for migration guide * update README * rst format * bullet * addressing PR review comments * use enums * remove a word
…-samples#1012) * Migrate quickstart to GAPIC client library * Migrate transcribe to GAPIC client library * Migrate transcribe_async to GAPIC client library * Migrate transcribe_streaming to GAPIC client library * clean up * clean up * Import from google.cloud.speech * update transcribe samples * import in alphabetic order * remove unused variable * use strings instead of enums * restructure code * comment on sreaming requests * import style * flake * correct indent * migrate transcribe_streaming_mic to gapic * update google-cloud-speech version requirement * addressing review comments * at the end of the audio stream, put None to signal to the generator * flake * addressing github review comments * add region tags for migration guide * update README * rst format * bullet * addressing PR review comments * use enums * remove a word
…-samples#1012) * Migrate quickstart to GAPIC client library * Migrate transcribe to GAPIC client library * Migrate transcribe_async to GAPIC client library * Migrate transcribe_streaming to GAPIC client library * clean up * clean up * Import from google.cloud.speech * update transcribe samples * import in alphabetic order * remove unused variable * use strings instead of enums * restructure code * comment on sreaming requests * import style * flake * correct indent * migrate transcribe_streaming_mic to gapic * update google-cloud-speech version requirement * addressing review comments * at the end of the audio stream, put None to signal to the generator * flake * addressing github review comments * add region tags for migration guide * update README * rst format * bullet * addressing PR review comments * use enums * remove a word
…-samples#1012) * Migrate quickstart to GAPIC client library * Migrate transcribe to GAPIC client library * Migrate transcribe_async to GAPIC client library * Migrate transcribe_streaming to GAPIC client library * clean up * clean up * Import from google.cloud.speech * update transcribe samples * import in alphabetic order * remove unused variable * use strings instead of enums * restructure code * comment on sreaming requests * import style * flake * correct indent * migrate transcribe_streaming_mic to gapic * update google-cloud-speech version requirement * addressing review comments * at the end of the audio stream, put None to signal to the generator * flake * addressing github review comments * add region tags for migration guide * update README * rst format * bullet * addressing PR review comments * use enums * remove a word
Updating python code samples to use the GAPIC python client library for Speech API.
Merge only after the client library is released.