Skip to content

Conversation

@ftnext
Copy link
Collaborator

@ftnext ftnext commented Nov 5, 2024

No description provided.

@ftnext ftnext self-assigned this Nov 5, 2024
@ftnext ftnext requested a review from Copilot November 19, 2025 15:29
Copilot finished reviewing on behalf of ftnext November 19, 2025 15:32
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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 PyAudioWrapper class in speech_recognition/microphone.py containing get_pyaudio() and list_microphone_names() methods
  • Removed list_microphone_names() from the Microphone class in speech_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():
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

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.

Suggested change
def list_microphone_names():
def list_microphone_names() -> list[str | None]:

Copilot uses AI. Check for mistakes.
Comment on lines +11 to +14
@patch("pyaudio.PyAudio")
def test_get_pyaudio(self, PyAudio):
assert PyAudioWrapper.get_pyaudio() == PyAudio.return_value
PyAudio.assert_called_once_with()
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

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.

Suggested change
@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()

Copilot uses AI. Check for mistakes.
Comment on lines +24 to +25
@staticmethod
def list_microphone_names():
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

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:

  1. Adding list_microphone_names = PyAudioWrapper.list_microphone_names as a class attribute in the Microphone class, or
  2. Importing and re-exporting the method from the microphone module in __init__.py

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant