fix:deprecate G2P #277
fix:deprecate G2P #277
Conversation
a companion PR will move it to ovos-audio it is just spammy and unused outside mk1
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... 📒 Files selected for processing (1)
WalkthroughThe pull request introduces significant modifications to the Changes
Possibly related PRs
Suggested labels
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
ovos_plugin_manager/templates/tts.py (2)
Line range hint
664-675: Remove the_cache_phonemesmethod to align with G2P deprecationThe method
_cache_phonemesis still present in theTTSclass, even though G2P functionality has been deprecated and moved toovos-audio. Keeping this method may cause confusion or unintended dependencies on deprecated functionality. It would be prudent to remove this method to fully encapsulate the deprecation.Apply this diff to remove the
_cache_phonemesmethod:- def _cache_phonemes(self, sentence, lang: str, cache: TextToSpeechCache = None, phonemes=None, sentence_hash=None): - """ - Caches phonemes for the given sentence. - - Args: - sentence (str): The sentence to cache phonemes for. - cache (TextToSpeechCache): The cache instance. - phonemes (str, optional): The phonemes for the sentence. - sentence_hash (str, optional): The hash of the sentence. - """ - sentence_hash = sentence_hash or hash_sentence(sentence) - if phonemes: - phoneme_file = cache.define_phoneme_file(sentence_hash) - phoneme_file.save(phonemes) - return phoneme_file - return None
Line range hint
1026-1038: Ensure deprecated classes are properly marked and alternatives providedClasses like
PlaybackThreadandRemoteTTSare marked for deprecation with upcoming removal. Make sure that all references to these classes in the codebase are updated to use their replacements. Additionally, confirm that any external documentation or examples are updated to guide users towards the new implementations.Consider adding deprecation warnings within these classes to alert users at runtime:
class PlaybackThread(Thread): """ PlaybackThread moved to ovos_audio.playback standalone plugin usage should rely on self.get_tts ovos-audio relies on self.execute and needs this class this class was only in ovos-plugin-manager in order to patch usage of our plugins in mycroft-core""" + def __init__(self, *args, **kwargs): + log_deprecation("PlaybackThread has been deprecated and will be removed in future releases. Please use the equivalent from ovos_audio.playback.", "1.0.0") + super().__init__(*args, **kwargs)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- ovos_plugin_manager/templates/tts.py (1 hunks)
🧰 Additional context used
🔇 Additional comments (2)
ovos_plugin_manager/templates/tts.py (2)
233-241: Deprecation ofg2pproperty is properly handledThe addition of the
g2pproperty and its setter correctly logs deprecation warnings, indicating that G2P functionality has moved toPlaybackThreadinovos-audio. Accessing or settingg2pwill inform users of this change without disrupting the current workflow.
Line range hint
531-537: Verify viseme generation without G2P dependenciesThe
_get_visemesmethod now relies solely on thephonemesprovided byself.get_tts. Since G2P functionality has been deprecated, ensure that all TTS plugins supply phoneme data as needed. This change could impact viseme generation if phonemes are not available from the TTS engine.Run the following script to identify TTS plugins that do not provide phoneme data:
✅ Verification successful
Let me try a different approach to verify this concern by searching for TTS plugins and their phoneme handling.
Let me try one more search focusing on the template and base class implementation to understand the phoneme handling requirements.
Based on the code analysis, I can now provide a final response about the viseme generation concern.
Viseme generation is safely handled with or without phoneme data
The code shows that the
_get_visemesmethod is designed to handle both cases where phonemes are available or not:
- If phonemes are provided by the TTS engine, they are converted to visemes using the
viseme()method- If phonemes are not available, an empty viseme list is returned with a debug log
- The
StreamingTTSbase class explicitly documents that it doesn't support phonemes and returnsNone- The caching system properly handles both audio and phoneme data independently
No changes are required as the system gracefully handles TTS plugins that don't provide phoneme data.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find TTS plugins that lack phoneme support. # Test: Search for TTS plugins without `phonemes` in their `get_tts` return statement. # Expectation: Identify plugins that need updates to provide phoneme data. fd -e py 'get_tts' ovos_tts_plugins/ | xargs grep -L 'return.*,.*phoneme'Length of output: 195
Script:
#!/bin/bash # First, let's find where TTS plugins are located in the repository fd -t d "tts" # Then let's examine the base TTS class implementation rg -A 5 "class TTSPlugin" # And look for get_tts method implementations ast-grep --pattern 'def get_tts(self, sentence, lang, *args, **kwargs): $$$ 'Length of output: 125
Script:
#!/bin/bash # Look for the base TTS class and get_tts method in the templates rg -A 10 "class TTS" ovos_plugin_manager/templates/tts.py # Search for phoneme-related code in the TTS template rg -B 5 -A 5 "phoneme" ovos_plugin_manager/templates/tts.py # Look for viseme-related code rg -B 5 -A 5 "_get_visemes" ovos_plugin_manager/templates/tts.pyLength of output: 12336
|
companion PR OpenVoiceOS/ovos-audio#109 |
a companion PR will move it to ovos-audio
it is just spammy and unused outside mk1
Summary by CodeRabbit