Skip to content

Generate Text to Speech#332

Merged
germanattanasio merged 25 commits intonextfrom
generate-text-to-speech
Feb 9, 2018
Merged

Generate Text to Speech#332
germanattanasio merged 25 commits intonextfrom
generate-text-to-speech

Conversation

@ammardodin
Copy link
Copy Markdown
Contributor

@ammardodin ammardodin commented Dec 12, 2017

Summary

  • Generated Text to Speech from the latest swagger
  • Added missing unit tests for get_voice
  • Updated method names in unit tests
  • Integration tests
  • Updated examples as per new .env file

@codecov-io
Copy link
Copy Markdown

codecov-io commented Dec 12, 2017

Codecov Report

Merging #332 into next will decrease coverage by 0.33%.
The diff coverage is 37.44%.

Impacted file tree graph

@@            Coverage Diff             @@
##             next     #332      +/-   ##
==========================================
- Coverage   38.39%   38.05%   -0.34%     
==========================================
  Files          25       25              
  Lines        8720     9141     +421     
==========================================
+ Hits         3348     3479     +131     
- Misses       5372     5662     +290
Impacted Files Coverage Δ
watson_developer_cloud/utils.py 100% <100%> (ø) ⬆️
watson_developer_cloud/text_to_speech_v1.py 38.37% <37.31%> (-61.63%) ⬇️
watson_developer_cloud/watson_service.py 77.68% <0%> (-0.42%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 401efc1...a08b726. Read the comment docs.

@ammardodin ammardodin requested a review from mkistler December 14, 2017 17:24
Copy link
Copy Markdown

@mkistler mkistler left a comment

Choose a reason for hiding this comment

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

Looks good but we need to be more careful about maintaining compatibility. We should also eliminate the duplicate accept parameter.


:param str text: The text to synthesize.
:param str accept: The requested audio format (MIME type) of the audio. You can use this header or the `accept` query parameter to specify the audio format. (For the `audio/l16` format, you can optionally specify `endianness=big-endian` or `endianness=little-endian`; the default is little endian.).
:param str accept2: The requested audio format (MIME type) of the audio. You can use this query parameter or the `Accept` header to specify the audio format. (For the `audio/l16` format, you can optionally specify `endianness=big-endian` or `endianness=little-endian`; the default is little endian.).
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We should ignore the accept query parameter -- it duplicates the function of the accept header parameter, and the header parameter is the better approach.

return self.request(method='GET', url='/v1/customizations',
params=params, accept_json=True)

def get_customization(self, customization_id):
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It will break compatibility to remove these methods, and I think that's a problem. Why not leave them? I don't think they conflict with any of the generated methods. Maybe we can mark them as deprecated, and add comments recommending the new method names. That will set us up to remove them in the next major release.

@mkistler
Copy link
Copy Markdown

We can use Python's warnings package to add deprecation warnings to existing methods that have different names in the generated SDK.

http://www.arruda.blog.br/programacao/python-use-warnings/

@germanattanasio germanattanasio changed the base branch from master to next February 7, 2018 18:07
Copy link
Copy Markdown

@mkistler mkistler left a comment

Choose a reason for hiding this comment

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

I have some misgivings about including beta methods in the main file of the SDK, but we may be down that road too far to change it now.

# Copyright 2016 IBM All Rights Reserved.
# coding: utf-8

# Copyright 2017 IBM All Rights Reserved.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copyright should be 2018 now, or '2017,2018'.

The v1 Text to Speech service
(https://www.ibm.com/watson/developercloud/text-to-speech.html)
For more information about the service and its various interfaces, see [About Text to
Speech](https://console.bluemix.net/docs/services/text-to-speech/index.html).
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The link to "console.bluemix.net" is going to be deprecated very soon.

Retrieves all voices available for speech synthesis.

Lists information about all available voices. To see information about a specific
voice, use the `GET /v1/voices/{voice}` method.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We need to work on removing the REST details from descriptions.

def pronunciation(self, text, voice=None, pronunciation_format='ipa'):
@deprecated('Use list_voices() instead')
def voices(self):
return self.list_voices()
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice!

You can request the pronunciation for a specific format. You can also request the
pronunciation for a specific voice to see the default translation for the language
of that voice or for a specific custom voice model to see the translation for that
voice model. **Note:** This method is currently a beta release.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should we exclude get_pronunciation from the SDK if it is still beta? Or should we mark it in some special way?

as defined in the model. To see just the metadata for a voice model, use the `GET
/v1/customizations` method. You must use credentials for the instance of the
service that owns a model to list information about it. **Note:** This method is
currently a beta release.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Another beta method.

you own for all languages. To see the words in addition to the metadata for a
specific voice model, use the `GET /v1/customizations/{customization_id}` method.
You must use credentials for the instance of the service that owns a model to list
information about it. **Note:** This method is currently a beta release.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Another beta method.

"""
Translation.

:attr str translation: The phonetic or sounds-like translation for the word. A phonetic translation is based on the SSML format for representing the phonetic string of a word either as an IPA translation or as an IBM SPR translation. A sounds-like is one or more words that, when combined, sound like the word.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Translation.translation is going to cause problems for C#.

"""
VoiceModels.

:attr list[VoiceModel] customizations: An array of `VoiceModel` objects that provides information about each available custom voice model. The array is empty if the requesting service credentials own no custom voice models (if no language is specified) or own no custom voice models for the specified language.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Seems like we want this to be named voiceModels and not customizations. I think this would be as simple as adding an x-alternate-name annotation to the swagger.

@germanattanasio germanattanasio merged commit ad44aac into next Feb 9, 2018
@germanattanasio germanattanasio deleted the generate-text-to-speech branch February 14, 2018 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

6 participants