Stop importing pyaudio as soon as the Microphone class is parsed #419
Conversation
The import is only used for type checking, so it can instead be put inside an `if TYPE_CHECKING:` block so the optional dependency isn't needed at all times.
|
Warning Walkthrough skippedFile diffs could not be summarized. 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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
|
I am going to keep this PR open until I have time to investigate this. I think In the meantime, I am going to just revert and disable the static checking to unblock everyone. |
![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 (1)
deepgram/audio/microphone/microphone.py (1)
[!TIP]
Codebase VerificationThe dynamic imports of
pyaudioin the__init__and_callbackmethods do not have any associated error handling. This could lead to issues ifpyaudiois not installed in the environment where the code is executed.
deepgram/audio/microphone/microphone.py: Line 53deepgram/audio/microphone/microphone.py: Line 131Analysis chain
Line range hint
53-53: Dynamic imports ofpyaudiowithin the__init__and_callbackmethods are correctly placed to ensure thatpyaudiois loaded only when necessary. However, ensure that there is adequate error handling around these imports to manage scenarios wherepyaudiomight not be installed in environments where it's expected to be used.Consider adding error handling for
pyaudioimports to gracefully handle its absence, especially since it's an optional dependency.Also applies to: 131-131
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for error handling around dynamic imports of `pyaudio`. # Test: Search for error handling related to `pyaudio` imports. rg --type python $'try:\n\s*import pyaudio\nexcept'Length of output: 242
Script:
#!/bin/bash # Description: Check for error handling around dynamic imports of `pyaudio`. # Test: Search for error handling related to `pyaudio` imports using multiline mode. rg --type python --multiline $'try:\n\s*import pyaudio\nexcept'Length of output: 93
Script:
#!/bin/bash # Description: Check for error handling around dynamic imports of `pyaudio`. # Test: Search for error handling related to `pyaudio` imports using multiline mode. rg --multiline $'try:\n\s*import pyaudio\nexcept'Length of output: 48
Script:
#!/bin/bash # Description: Verify the presence of dynamic imports of `pyaudio` and check their context for error handling. # Search for dynamic imports of `pyaudio` in all Python files. rg 'import pyaudio' --context 5Length of output: 2543
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- deepgram/audio/microphone/microphone.py (2 hunks)
Files not summarized due to errors (1)
- deepgram/audio/microphone/microphone.py: Error: Server error. Please try again later.
Additional comments not posted (1)
deepgram/audio/microphone/microphone.py (1)
14-16: The conditional import ofpyaudioinside theTYPE_CHECKINGblock is a good practice to avoid unnecessary dependencies during runtime. This change aligns well with the objective of makingpyaudioan optional import, thus resolving the issue reported.
|
It was added in Python 3.5.2, according to the documentation: https://docs.python.org/3/library/typing.html#typing.TYPE_CHECKING. |
|
Ok. Will give this a ok then. Thanks for the help! |
|
@Tenzer thanks again for the PR. just posted a new release with this fix: |
|
Thanks! I can see the new version passes tests in our application at least, so that's better than with version 3.3.0 :) |
Proposed changes
The import is only used for type checking, so it can instead be put inside an
if TYPE_CHECKING:block so the optional dependency isn't needed at all times.Types of changes
What types of changes does your code introduce to the community Python SDK?
Put an
xin the boxes that applyChecklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.Further comments
Fixes #418.