fix/restore_dead_code #256
Conversation
with deprecation warnings
WalkthroughThe changes involve the deprecation of several methods in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TTS
User->>TTS: Access properties
TTS-->>User: Return properties directly
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: 4
Outside diff range, codebase verification and nitpick comments (2)
ovos_plugin_manager/templates/tts.py (2)
136-141: Deprecated Methodget_messageStill in UseThe
get_messagemethod, which has been deprecated, is still being used in the following locations within theovos_plugin_manager/templates/tts.pyfile. These usages should be refactored to ensure the deprecation is complete:
- Line 1:
message = self.get_message(kwargs)- Line 2:
message = self.get_message(kwargs)Please update these references to prevent potential runtime issues.
Analysis chain
Review of Deprecated Method:
get_messageThe method
get_messagehas been deprecated without a direct replacement. This approach is acceptable if the functionality is no longer needed or if it's being replaced by a more efficient mechanism elsewhere in the codebase. Ensure that all references to this method are removed or updated to prevent runtime errors.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `get_message` are removed or updated. # Test: Search for the function usage. Expect: No occurrences of the old usage. rg --type python -A 5 $'get_message'Length of output: 66
Script:
#!/bin/bash # Description: Verify all function calls to `get_message` are removed or updated. # Test: Search for the function usage. Expect: No occurrences of the old usage. rg -A 5 'get_message'Length of output: 1558
155-163: Deprecated Method Usage Detected:get_voiceThe method
get_voiceis still being used in the codebase, specifically in the following location. It should be replaced withself.voiceto avoid runtime issues and align with the deprecation notice.
ovos_plugin_manager/templates/tts.py: Line withreturn self.get_lang(kwargs), self.get_voice(kwargs)Analysis chain
Review of Deprecated Method:
get_voiceThe method
get_voicehas been deprecated with a suggestion to accessself.voicedirectly. This is a straightforward simplification that should help reduce the complexity of the code. Confirm that all usages ofget_voiceare replaced with direct property access to avoid any runtime issues.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `get_voice` are updated to use `self.voice`. # Test: Search for the function usage. Expect: Only occurrences of the new property access. rg --type python -A 5 $'get_voice'Length of output: 64
Script:
#!/bin/bash # Description: Verify all function calls to `get_voice` are updated to use `self.voice`. # Corrected Test: Search for the function usage in Python files. rg --glob '*.py' -A 5 'get_voice'Length of output: 5506
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- ovos_plugin_manager/templates/tts.py (3 hunks)
- ovos_plugin_manager/utils/init.py (2 hunks)
- test/unittests/test_tts.py (3 hunks)
Additional context used
Ruff
test/unittests/test_tts.py
122-122:
ovos_plugin_manager.templates.tts.TTSValidatorimported but unusedRemove unused import:
ovos_plugin_manager.templates.tts.TTSValidator(F401)
179-179:
ovos_plugin_manager.tts.get_voice_idimported but unusedRemove unused import:
ovos_plugin_manager.tts.get_voice_id(F401)
183-183:
ovos_plugin_manager.tts.scan_voicesimported but unusedRemove unused import:
ovos_plugin_manager.tts.scan_voices(F401)
187-187:
ovos_plugin_manager.tts.get_voicesimported but unusedRemove unused import:
ovos_plugin_manager.tts.get_voices(F401)
Additional comments not posted (4)
ovos_plugin_manager/utils/__init__.py (1)
201-203: Approve the addition of the deprecated parameter.The addition of the
chop_samplesparameter, marked as deprecated, is handled correctly with a deprecation warning. This maintains backward compatibility while discouraging its use in new code. However, it would be beneficial to document this deprecation more clearly in the class docstring to ensure that it is visible to developers who might not look at the constructor's implementation directly.ovos_plugin_manager/templates/tts.py (3)
146-153: Review of Deprecated Method:get_genderThe method
get_genderhas been deprecated with a suggestion to accessself.voiceandself.langdirectly. This change is aimed at simplifying property access within the class. Verify that the transition to the new access pattern does not break existing functionality, especially where gender-specific logic might be applied.
142-145: Review of Deprecated Method:get_langThe method
get_langhas been deprecated with a suggestion to accessself.langdirectly. This change simplifies the code and reduces unnecessary method calls. Ensure that all instances whereget_langwas used are updated to directly access theself.langproperty.Verification successful
Verification Successful: Deprecated Method
get_langThe method
get_langis not used elsewhere in the codebase, confirming that all instances have been updated to directly accessself.langas suggested. No further action is needed.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `get_lang` are updated to use `self.lang`. # Test: Search for the function usage. Expect: Only occurrences of the new property access. rg --type python -A 5 $'get_lang'Length of output: 63
Script:
#!/bin/bash # Description: Verify all function calls to `get_lang` are updated to use `self.lang`. # Test: Search for the function usage. Expect: Only occurrences of the new property access. rg --type py -A 5 'get_lang'Length of output: 26111
165-168: Review of Deprecated Method:getThe method
gethas been deprecated with a suggestion to accessself.voiceandself.langdirectly. This method previously returned a tuple, so ensure that the refactoring does not affect the expected data structure in calling code. Update all relevant parts of the codebase to align with this new access pattern.Verification successful
Verification of Deprecated Method:
getThe
getmethod inovos_plugin_manager/templates/tts.pyis not being used elsewhere in the codebase, or its usage has already been refactored to align with the new access pattern of usingself.voiceandself.langdirectly. No further action is required.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `get` are updated to use `self.voice` and `self.lang`. # Test: Search for the function usage. Expect: Only occurrences of the new property access. rg --type python -A 5 $'get'Length of output: 58
Script:
#!/bin/bash # Description: Verify all function calls to `get` are updated to use `self.voice` and `self.lang`. # Test: Search for the function usage. Expect: Only occurrences of the new property access. rg --type py -A 5 $'get'Length of output: 332342
| # TODO | ||
|
|
||
| def test_scan_voices(self): | ||
| pass | ||
| from ovos_plugin_manager.tts import scan_voices |
There was a problem hiding this comment.
Remove unused import: scan_voices
The import from ovos_plugin_manager.tts import scan_voices is unused in the current context. This aligns with the static analysis tool's suggestion to remove it to clean up the codebase.
Apply this diff to remove the unused import:
- from ovos_plugin_manager.tts import scan_voicesCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from ovos_plugin_manager.tts import scan_voices |
Tools
Ruff
183-183:
ovos_plugin_manager.tts.scan_voicesimported but unusedRemove unused import:
ovos_plugin_manager.tts.scan_voices(F401)
| # TODO | ||
|
|
||
| def test_get_voices(self): | ||
| pass | ||
| from ovos_plugin_manager.tts import get_voices |
There was a problem hiding this comment.
Remove unused import: get_voices
The import from ovos_plugin_manager.tts import get_voices is unused in the current context. This aligns with the static analysis tool's suggestion to remove it to clean up the codebase.
Apply this diff to remove the unused import:
- from ovos_plugin_manager.tts import get_voicesCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from ovos_plugin_manager.tts import get_voices |
Tools
Ruff
187-187:
ovos_plugin_manager.tts.get_voicesimported but unusedRemove unused import:
ovos_plugin_manager.tts.get_voices(F401)
|
|
||
| def test_remote_tt(self): | ||
| pass | ||
| from ovos_plugin_manager.templates.tts import TTSValidator |
There was a problem hiding this comment.
Remove unused import: TTSValidator
The import from ovos_plugin_manager.templates.tts import TTSValidator is unused in the current context. This aligns with the static analysis tool's suggestion to remove it to clean up the codebase.
Apply this diff to remove the unused import:
- from ovos_plugin_manager.templates.tts import TTSValidatorCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from ovos_plugin_manager.templates.tts import TTSValidator |
Tools
Ruff
122-122:
ovos_plugin_manager.templates.tts.TTSValidatorimported but unusedRemove unused import:
ovos_plugin_manager.templates.tts.TTSValidator(F401)
| @@ -193,15 +176,15 @@ | |||
| self.CONFIG_SECTION, None) | |||
|
|
|||
| def test_get_voice_id(self): | |||
| pass | |||
| from ovos_plugin_manager.tts import get_voice_id | |||
There was a problem hiding this comment.
Remove unused import: get_voice_id
The import from ovos_plugin_manager.tts import get_voice_id is unused in the current context. This aligns with the static analysis tool's suggestion to remove it to clean up the codebase.
Apply this diff to remove the unused import:
- from ovos_plugin_manager.tts import get_voice_idCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from ovos_plugin_manager.tts import get_voice_id |
Tools
Ruff
179-179:
ovos_plugin_manager.tts.get_voice_idimported but unusedRemove unused import:
ovos_plugin_manager.tts.get_voice_id(F401)
with deprecation warnings per #249 review
Summary by CodeRabbit
New Features
Bug Fixes
Documentation