build: simplify dependencies #301
Conversation
maciejmajek
force-pushed
the
build/simplify-deps
branch
from
e3a1943
to
c94dbdf
Compare
|
@coderabbitai full review |
![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)
pyproject.toml (2)
67-71: LGTM! Consider using a more specific version constraint
The addition of the optional local_asr group aligns well with the PR objective of simplifying dependencies. The group is properly configured as optional, which will help reduce the default installation size.
Consider using a more specific version constraint for openai-whisper to ensure reproducible builds:
-openai-whisper = "^20231117"
+openai-whisper = "==20231117"Line range hint 1-72: Document optional dependency groups in README
The reorganization of dependencies into optional groups is a good architectural decision that will help users install only what they need. To make this more user-friendly, consider:
- Adding a section in the README that explains:
- Available optional dependency groups
- How to install specific groups (e.g.,
poetry install --with local_asr) - Which features require which groups
- Updating CI/CD pipelines to test different dependency combinations
Would you like me to help draft the documentation for these optional groups?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
- pyproject.toml (1 hunks)
🔇 Additional comments (1)
pyproject.toml (1)
Line range hint 14-52: Verify impact of removing torchaudio dependency
The removal of torchaudio from main dependencies could impact existing users. While its functionality might be replaced by openai-whisper in the new local_asr group, we should verify there are no direct dependencies on torchaudio in the codebase.
✅ Verification successful
The initial scan found no direct imports of torchaudio, but we found several audio-related files. Let's examine their implementations to understand if they rely on torchaudio indirectly or if the audio processing is handled through other libraries.
Based on the examination of the audio-related files, I can now generate the final response:
Removing torchaudio dependency is safe
The codebase does not rely on torchaudio for audio processing. Instead, it uses:
scipy.io.wavfilefor audio file operationsopenai-whisperfor speech recognitionelevenlabsand OpenTTS for text-to-speech
The removal of torchaudio aligns with the goal of simplifying dependencies without breaking existing functionality.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any direct usage of torchaudio in the codebase
echo "Searching for torchaudio imports and usage..."
rg "import.*torchaudio|from.*torchaudio" -l
# Search for any potential audio processing related files
echo -e "\nSearching for potential audio-related files..."
fd -e py -x grep -l "audio" {}
Length of output: 562
Script:
#!/bin/bash
# Let's check the content of audio-related files to understand the audio processing implementation
echo "=== Checking ASR implementation ==="
cat ./src/rai_asr/rai_asr/asr_clients.py
echo -e "\n=== Checking TTS implementation ==="
cat ./src/rai_tts/rai_tts/tts_clients.py
echo -e "\n=== Checking setup.py for ASR dependencies ==="
cat ./src/rai_asr/setup.py
Length of output: 8065
WalkthroughThe changes involve updates to the Changes
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 (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
pyproject.toml (1)
Line range hint 13-72: Consider further dependency optimization opportunities.
While this PR makes a good start at simplifying dependencies, there are additional opportunities for optimization:
- Consider grouping LangChain-related dependencies (
langchain*) into an optional group for users not requiring LLM features - The use of wildcards (
*) in version constraints for packages likelangchainandlanggraphcould lead to unexpected updates
Consider creating a new optional group for LangChain dependencies:
+[tool.poetry.group.llm]
+optional = true
+
+[tool.poetry.group.llm.dependencies]
+langchain-core = "^0.3"
+langchain = "^0.1"
+langgraph = "^0.0.10"
+langchain-aws = "^0.0.3"
+langchain-openai = "^0.0.5"
+langchain-ollama = "^0.0.5"
+langchain-community = "^0.0.10"This would allow users to install the base package without LLM capabilities when not needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (1)
- pyproject.toml (1 hunks)
🔇 Additional comments (2)
pyproject.toml (2)
Line range hint 1-72: Verify the removal of torchaudio dependency.
While moving openai-whisper to an optional group is well-handled, the removal of torchaudio needs verification. This package might be required by other dependencies or functionality.
Let's check for potential torchaudio usage:
#!/bin/bash
# Search for torchaudio imports or usage
echo "=== Checking for direct torchaudio usage ==="
rg "import torchaudio|from torchaudio" -l
echo "=== Checking if torchaudio is a dependency of other packages ==="
rg "torchaudio" "requirements*.txt" "setup*.py" "pyproject.toml" || true67-71: LGTM! Good separation of ASR dependencies.
The addition of the local_asr optional group aligns well with the PR objective of simplifying dependencies. This change allows users to install ASR capabilities only when needed.
Let's verify there are no direct dependencies on whisper in the codebase that might break without this optional dependency:
✅ Verification successful
Let me try another search strategy to verify whisper usage:
LGTM! The optional dependency is properly handled in the codebase.
The verification shows that the whisper dependency is well-managed:
- The ASR functionality is properly isolated in the
rai_asrpackage - The code has proper conditional imports in
asr_node.pythat handle both local whisper and OpenAI whisper options - Launch files provide configuration flexibility through the
asr_vendorparameter - Documentation clearly indicates whisper as an optional dependency
The move to make whisper an optional dependency is safe and aligns with the PR's goal of simplifying installation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for whisper imports or usage that might break without the optional dependency
rg -l "import whisper|from whisper" | while read -r file; do
echo "=== $file ==="
rg "import whisper|from whisper" "$file" -A 5
done
Length of output: 459
Script:
#!/bin/bash
# Search for whisper-related patterns more broadly
echo "=== Searching for whisper patterns ==="
rg -i "whisper" -l
echo -e "\n=== Checking content of files with whisper mentions ==="
rg -i "whisper" -C 5
Length of output: 22453
Purpose
RAI's default python environment takes a lot of time to install. Some of the dependencies should be marked as optional.
Proposed Changes
Moved non-core dependencies to optional category
Issues
Testing
Summary by CodeRabbit
New Features
openai-whisper.Bug Fixes
torchaudiodependency to streamline performance.