-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Use Transcript object in sync and async. #2613
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
Use Transcript object in sync and async. #2613
Conversation
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.
Mostly LGTM, though ping me once issues resolved
@@ -23,6 +23,7 @@ | |||
from google.cloud.speech.encoding import Encoding | |||
from google.cloud.speech.operation import Operation | |||
from google.cloud.speech.sample import Sample | |||
from google.cloud.speech.transcript import Transcript |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
return api_response['results'][0]['alternatives'] | ||
return [Transcript.from_api_repr(alternative) | ||
for alternative | ||
in api_response['results'][0]['alternatives']] |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
def from_api_repr(cls, transcript): | ||
"""Factory: construct ``Transcript`` from JSON response. | ||
|
||
:type transcript: :class:`~SpeechRecognitionAlternative` |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
:rtype: :class:`~Transcript` | ||
:returns: Instance of ``Transcript``. | ||
""" | ||
return cls(transcript['transcript'], transcript['confidence']) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
self.assertEqual(response, expected) | ||
alternative = SYNC_RECOGNIZE_RESPONSE['results'][0]['alternatives'][0] | ||
expected = [Transcript.from_api_repr(alternative)] | ||
self.assertIsInstance(response[0], Transcript) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
expected = [Transcript.from_api_repr(alternative)] | ||
self.assertIsInstance(response[0], Transcript) | ||
self.assertEqual(response[0].transcript, expected[0].transcript) | ||
self.assertEqual(response[0].confidence, expected[0].confidence) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
SYNC_RECOGNIZE_RESPONSE['results'][0]['alternatives'][0])] | ||
self.assertIsInstance(response[0], Transcript) | ||
self.assertEqual(response[0].transcript, expected[0].transcript) | ||
self.assertEqual(response[0].confidence, expected[0].confidence) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
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.
LGTM pending my two nits and length checks. Feel free to incorporate my feedback about the results[0]
bit, but don't feel required to.
@@ -112,10 +112,11 @@ def test_sync_recognize_content_with_optional_parameters(self): | |||
self.assertEqual(req['path'], 'speech:syncrecognize') | |||
|
|||
alternative = SYNC_RECOGNIZE_RESPONSE['results'][0]['alternatives'][0] | |||
expected = [Transcript.from_api_repr(alternative)] | |||
expected = Transcript.from_api_repr(alternative) | |||
self.assertEqual(len(response, 1)) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
if len(api_response['results']) == 1: | ||
results = api_response['results'] | ||
if len(results) == 1: | ||
result = results[0]['alternatives'] |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
expected = [Transcript.from_api_repr( | ||
SYNC_RECOGNIZE_RESPONSE['results'][0]['alternatives'][0])] | ||
expected = Transcript.from_api_repr( | ||
SYNC_RECOGNIZE_RESPONSE['results'][0]['alternatives'][0]) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
c90f33a
to
325cbe7
Compare
Ok thanks! I changed the response check via your suggestion, added the length check and fixed my bug. I also just squashed the commits. |
Travis is green. Merging. |
Use Transcript object in sync and async.
Use Transcript object in sync and async.
Use Transcript object in sync and async.
…les#2613) * automl: add base samples * automl: add base set of samples * Clean up tests * License year 2020, drop python2 print statement unicode * use centralized automl testing project * Fix GCS path typo * Use fake dataset for batch predict * lint: line length * fix fixture naming and use * Fix fixture changes * Catch resource exhausted error * use fake data for import test * update how to access an operation id Co-authored-by: Leah E. Cole <[email protected]>
Use Transcript object in sync and async.
In the spirit of smaller PRs. I started switching sync to GAPIC again and I realized that this is required first.