Conversation
|
Hi @Orbiter, please review this PR changes. |
There was a problem hiding this comment.
Pull request overview
Introduces uv-based Python dependency management for the project, aiming to make environment setup reproducible and to codify supported Python/dependency versions.
Changes:
- Remove
requirements.txtin favor of PEP 621 dependencies inpyproject.toml. - Add
pyproject.tomlwithrequires-pythonand declared dependencies (Django/Flask + supporting libs). - Update setup docs (
README.md,django/HACKING.md) to useuv sync, and ignore.venv/in.gitignore.
Reviewed changes
Copilot reviewed 3 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| requirements.txt | Removes legacy dependency list (moves dependency management to uv/pyproject.toml). |
| pyproject.toml | Adds Python version constraints and dependency declarations for uv. |
| django/HACKING.md | Updates Django developer setup steps to use uv workflow. |
| README.md | Updates repository quickstart instructions to use uv. |
| .gitignore | Adds .venv/ to prevent committing local virtual environments. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "requests>=2.32.5", | ||
| "scipy>=1.17.1", | ||
| "torch>=2.10.0", | ||
| "whisper>=1.1.10", |
There was a problem hiding this comment.
pyproject.toml includes the whisper PyPI package, but the repo already depends on openai-whisper (which provides the whisper module used by the code). Installing both can cause the wrong distribution to be imported at runtime (and both are currently resolved in uv.lock). Remove the extra whisper dependency (or switch fully to it and drop openai-whisper + adjust code) to avoid module name collisions.
| "whisper>=1.1.10", |
| * Run `audio_grabber.py` to start capturing audio from the microphone | ||
| * Run `transcribe_server.py` to start the server | ||
| * Open `transcribe_listener.py` in the browser to start displaying transcribed text in real-time |
There was a problem hiding this comment.
README setup steps reference files that don’t exist at the repo root: audio_grabber.py and transcribe_server.py live under flask/, and the listener is flask/transcribe_listener.html (not transcribe_listener.py). Update these paths/filenames so the quickstart instructions work.
| * Run `audio_grabber.py` to start capturing audio from the microphone | |
| * Run `transcribe_server.py` to start the server | |
| * Open `transcribe_listener.py` in the browser to start displaying transcribed text in real-time | |
| * Run `flask/audio_grabber.py` to start capturing audio from the microphone | |
| * Run `flask/transcribe_server.py` to start the server | |
| * Open `flask/transcribe_listener.html` in the browser to start displaying transcribed text in real-time |
| * install uv: https://docs.astral.sh/uv/getting-started/installation/ | ||
| * Install Venv by running: | ||
| ``` | ||
| uv sync | ||
| ``` |
There was a problem hiding this comment.
The README adds uv setup but still doesn’t document the supported Python version range, even though the PR is meant to address Python version issues. Consider explicitly stating the supported range (matching requires-python in pyproject.toml) near the uv sync instructions so users don’t try unsupported versions.
| ### 2. Create a Virtual Environment and Install Dependencies | ||
|
|
||
| ``` | ||
| python3 -m venv venv | ||
| source venv/bin/activate # On Windows use `venv\Scripts\activate` | ||
| ``` | ||
| To set up Virtual Environment and install dependencies, follow these steps: | ||
|
|
There was a problem hiding this comment.
django/HACKING.md now instructs using uv sync, but the earlier “Project Structure” section still lists requirements.txt even though the repo no longer contains it. Update that structure snippet (and any other references) to reflect the pyproject.toml/uv.lock workflow.
| ### 2. Create a Virtual Environment and Install Dependencies | ||
|
|
||
| ``` | ||
| python3 -m venv venv | ||
| source venv/bin/activate # On Windows use `venv\Scripts\activate` | ||
| ``` | ||
| To set up Virtual Environment and install dependencies, follow these steps: | ||
|
|
There was a problem hiding this comment.
The prerequisites section says “Python 3.8 or higher”, but pyproject.toml specifies requires-python = ">=3.11, <3.14". Please align the documented prerequisite Python versions with the new requires-python constraint to avoid users creating unsupported environments.
| - WHISPER_SERVER_USE: Set to false to use local models. | ||
| - WHISPER_MODEL_FAST: The name of the fast Whisper model (e.g., small). | ||
| - WHISPER_MODEL_SMART: The name of the smart Whisper model (e.g., medium). |
There was a problem hiding this comment.
The documented env vars don’t match the implementation: this doc references WHISPER_MODEL_FAST / WHISPER_MODEL_SMART, but the Django/Flask code reads WHISPER_MODEL for both model names. Update the documentation to match the code, or update the code to honor the documented variables.
What is this PR introduce?
fix dependencies and python-version issue
What is the previous behaviour?
What is the new Behaviour?
Related Issue:
#5, #6, #12, #13