refactor: Extract PyAudioWrapper.list_microphone_names()#788
refactor: Extract PyAudioWrapper.list_microphone_names()#788ftnext wants to merge 9 commits intoUberi:masterfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR extracts the list_microphone_names() method from the Microphone class into a new PyAudioWrapper class in a dedicated microphone.py module. This refactoring aims to better organize microphone-related functionality and reduce coupling.
Key Changes
- Created a new
PyAudioWrapperclass inspeech_recognition/microphone.pycontainingget_pyaudio()andlist_microphone_names()methods - Removed
list_microphone_names()from theMicrophoneclass inspeech_recognition/__init__.py - Added initial test coverage for
PyAudioWrapper.get_pyaudio()in a new test file
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| speech_recognition/microphone.py | New module introducing PyAudioWrapper class with extracted microphone utility methods |
| speech_recognition/init.py | Removed list_microphone_names() method from Microphone class |
| tests/test_microphone.py | New test file with initial test for PyAudioWrapper.get_pyaudio() |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return pyaudio.PyAudio() | ||
|
|
||
| @staticmethod | ||
| def list_microphone_names(): |
There was a problem hiding this comment.
Missing return type annotation. The method should include -> list or more specifically -> list[str | None] to match the docstring which states it returns "a list of the names" where entries can contain None.
| def list_microphone_names(): | |
| def list_microphone_names() -> list[str | None]: |
| @patch("pyaudio.PyAudio") | ||
| def test_get_pyaudio(self, PyAudio): | ||
| assert PyAudioWrapper.get_pyaudio() == PyAudio.return_value | ||
| PyAudio.assert_called_once_with() |
There was a problem hiding this comment.
The patch target is incorrect. Since get_pyaudio() imports pyaudio locally within the function (line 17 in microphone.py), patching pyaudio.PyAudio at the module level won't work. The patch should target speech_recognition.microphone.pyaudio.PyAudio or the import within the function itself. Consider using @patch("speech_recognition.microphone.pyaudio") and mocking the entire module.
| @patch("pyaudio.PyAudio") | |
| def test_get_pyaudio(self, PyAudio): | |
| assert PyAudioWrapper.get_pyaudio() == PyAudio.return_value | |
| PyAudio.assert_called_once_with() | |
| @patch("speech_recognition.microphone.pyaudio") | |
| def test_get_pyaudio(self, mock_pyaudio): | |
| assert PyAudioWrapper.get_pyaudio() == mock_pyaudio.PyAudio.return_value | |
| mock_pyaudio.PyAudio.assert_called_once_with() |
| @staticmethod | ||
| def list_microphone_names(): |
There was a problem hiding this comment.
The refactoring breaks the public API. The Microphone.list_microphone_names() method is documented and used in examples (see README.rst line 221 and reference/library-reference.rst). After this change, calling sr.Microphone.list_microphone_names() will fail with an AttributeError. Consider either:
- Adding
list_microphone_names = PyAudioWrapper.list_microphone_namesas a class attribute in the Microphone class, or - Importing and re-exporting the method from the microphone module in
__init__.py
|
@codex review |
No description provided.