refactor: consolidate dependencies into pyproject.toml #583
Conversation
📝 WalkthroughWalkthroughConsolidates dependency management into pyproject.toml, removes Changes
Sequence Diagram(s)(omitted — changes are packaging, docs, CI, and tests; no new multi-component runtime control-flow suitable for a sequence diagram) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @divanshu-go, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors the project's Python dependency management by centralizing all package declarations in Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is an excellent refactoring that modernizes the project's dependency management by consolidating all dependencies into pyproject.toml. This removes legacy files like setup.py and requirements.txt, eliminates duplication, and resolves inconsistencies, making dependency management more robust and easier to maintain. The documentation has been updated consistently across the repository to reflect these changes. My review includes a few minor suggestions to further improve documentation clarity and code maintainability.
There was a problem hiding this comment.
Pull request overview
This PR modernizes Python packaging by consolidating dependencies into pyproject.toml as the single source of truth, following PEP 518/621 standards. It eliminates the duplication and maintenance overhead from having dependencies scattered across setup.py, requirements.txt, and requirements-dev.txt. The changes also fix a critical issue where pip install -e . was missing dependencies like requests, cryptography, and typing-extensions.
Changes:
- Consolidated all dependencies into
pyproject.tomlwith proper categorization (core, dev, security, docs) - Removed legacy files (
setup.py,requirements.txt,requirements-dev.txt) - Updated all documentation and CI workflows to use the new
pip install -e ".[dev]"pattern
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| setup.py | Removed deprecated setup.py; functionality now in pyproject.toml |
| requirements.txt | Removed; core dependencies moved to pyproject.toml (eliminated PyYAML duplicates) |
| requirements-dev.txt | Removed; dev dependencies moved to pyproject.toml optional-dependencies |
| pyproject.toml | Added missing core dependencies and consolidated all dev dependencies |
| docs/guides/Getting-Started.md | Updated installation instructions to use pyproject.toml |
| docs/guides/Developer-Guide.md | Updated dev setup to use pip install -e ".[dev]" |
| README.md | Updated installation instructions with new recommended approach |
| CONTRIBUTING.md | Updated contributor setup documentation |
| AGENTS.md | Updated agent testing setup instructions |
| Makefile | Simplified dev target to single install command |
| .github/workflows/ci.yml | Updated to use pyproject.toml for dependencies and cache key |
| .github/workflows/automation.yml | Updated to use pip install -e ".[dev]" |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @CONTRIBUTING.md:
- Around line 126-138: The comment incorrectly states that pip install -e
".[dev]" "includes all dependencies"; update the text to say that pip install -e
".[dev]" installs core dependencies plus development tooling only (not extras
like docs or security), and add a clear note that pip install -e ".[all]"
installs all extras; specifically modify the line referencing pip install -e
".[dev]" and the surrounding explanatory sentence to reflect this distinction
and mention pip install -e ".[all]" for full extras.
🧹 Nitpick comments (3)
README.md (1)
97-101: Clarify the installation comment for better user understanding.The comment "# Using pyproject.toml (recommended)" on line 97 may be confusing because both installation options use pyproject.toml. The actual distinction is between basic installation (runtime dependencies only) and development installation (includes dev/test tools).
📝 Suggested clarification
-# 3. Install Cortex -# Using pyproject.toml (recommended) +# 3. Install Cortex (basic installation) pip install -e . -# Or install with dev dependencies +# Or install with development dependencies (for contributors) pip install -e ".[dev]"docs/guides/Developer-Guide.md (1)
13-15: Preferpython -m piphere to avoid “wrong pip” installs.Using
pip install -e ".[dev]"is fine, but docs are more robust withpython -m pip ...so it always targets the active venv interpreter.Proposed doc tweak
-# Install dev dependencies -pip install -e ".[dev]" +# Install dev dependencies +python -m pip install -e ".[dev]"pyproject.toml (1)
45-60: Consider adding an upper bound fortyping-extensions(and confirm the new minimum versions exist/are intended).
requests>=2.32.4(released Jun 9, 2025) andcryptography>=42.0.0(released Jan 23, 2024) exist, so the new minimums shouldn’t break installs. (releasealert.dev)
Fortyping-extensions, upstream explicitly calls out semver from 4.0.0 onward and suggests depending within a major range—so>=4.0.0without<5may eventually allow a breaking5.xupgrade into your envs. (pypi.org)Proposed change
dependencies = [ @@ - # Type hints for older Python versions - "typing-extensions>=4.0.0", + # Type-hint backports; keep within major version (semver) + "typing-extensions>=4.0.0,<5", ]
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.github/workflows/automation.yml.github/workflows/ci.ymlAGENTS.mdCONTRIBUTING.mdMakefileREADME.mddocs/guides/Developer-Guide.mddocs/guides/Getting-Started.mdpyproject.tomlrequirements-dev.txtrequirements.txtsetup.py
💤 Files with no reviewable changes (3)
- requirements.txt
- requirements-dev.txt
- setup.py
🧰 Additional context used
📓 Path-based instructions (1)
{setup.py,setup.cfg,pyproject.toml,**/__init__.py}
📄 CodeRabbit inference engine (AGENTS.md)
Use Python 3.10 or higher as the minimum supported version
Files:
pyproject.toml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build Package
- GitHub Check: Agent
- GitHub Check: test (3.10)
- GitHub Check: test (3.12)
🔇 Additional comments (6)
AGENTS.md (1)
40-40: LGTM! Dev install command properly consolidated.The change correctly consolidates the development environment setup into a single command using dev extras, aligning with the pyproject.toml-based approach.
docs/guides/Getting-Started.md (1)
15-16: All runtime dependencies are properly declared inpyproject.toml. The packagesrequests(2.32.4),cryptography(42.0.0), andtyping-extensions(4.0.0) are present in the[project.dependencies]section, along with all other required runtime dependencies. The editable install command will correctly resolve these dependencies.Makefile (1)
19-22: Good switch to dev extras via the project’s interpreter.
$(PYTHON) -m pip install -e ".[dev]"is consistent with the pyproject-as-authority goal and avoids pip/venv mismatch issues..github/workflows/automation.yml (1)
28-32: Workflow simplification looks right; ensuredevstays the authoritative CI/security set.Installing via
pip install -e ".[dev]"in both testing and safety scanning is consistent with the PR goal, but it makes CI correctness hinge onpyproject.toml’sdevextras staying complete.Also applies to: 96-99
.github/workflows/ci.yml (1)
59-72: No action needed. The code is correctly configured.The
pip install -e ".[dev]"workflow step is properly supported:
[project.optional-dependencies].devincludes all required CI dependencies (pytest, pytest-cov, pytest-timeout, etc.)- The build backend is
setuptools.build_metawith setuptools>=61.0, which fully supports PEP 660 editable installspyproject.toml (1)
63-75: Dev extra additions look good; please verifypytest/pytest-asynciocompatibility in your CI matrix.Given
pytest-asynciohas had multiple behavioral shifts over time, it’s worth confirmingpytest>=7.0.0+pytest-asyncio>=0.21.0works across your supported Python versions (3.10–3.12) withasyncio_mode = "auto".
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/integration/test_end_to_end.py (1)
20-21: Quote the extras specifier to prevent shell glob expansion.The
[dev]in/workspace[dev]is a shell glob pattern that could be interpreted as a character class matchingd,e, orv. While this often works in containers with default bash settings, it's fragile. Quoting the path ensures consistent behavior.Proposed fix
PIP_BOOTSTRAP = "python -m pip install --quiet --upgrade pip setuptools build && python -m pip install --quiet --no-cache-dir -e /workspace" -PIP_BOOTSTRAP_DEV = "python -m pip install --quiet --upgrade pip setuptools build && python -m pip install --quiet --no-cache-dir -e /workspace[dev]" +PIP_BOOTSTRAP_DEV = "python -m pip install --quiet --upgrade pip setuptools build && python -m pip install --quiet --no-cache-dir -e '/workspace[dev]'"
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/integration/test_end_to_end.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Follow PEP 8 style guide
Type hints required in Python code
Docstrings required for all public APIs
Files:
tests/integration/test_end_to_end.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Maintain >80% test coverage for pull requests
Files:
tests/integration/test_end_to_end.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build Package
- GitHub Check: test (3.12)
- GitHub Check: test (3.11)
- GitHub Check: test (3.10)
🔇 Additional comments (1)
tests/integration/test_end_to_end.py (1)
106-126: LGTM!The test properly uses
PIP_BOOTSTRAP_DEVto install dev dependencies (pytest) before running the test suite. The comment on line 113 clearly explains the rationale. The structure follows the same pattern as other tests in the class.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
pyproject.toml (1)
58-59: Minor: Comment is slightly misleading.The comment "Type hints for older Python versions" is inaccurate since the project requires Python ≥3.10.
typing-extensionsactually backports newer typing features (e.g.,Self,TypeGuard,@override) from Python 3.11+ to 3.10.Consider updating to:
- # Type hints for older Python versions + # Backported typing features from newer Python versions "typing-extensions>=4.0.0",
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pyproject.toml
🧰 Additional context used
📓 Path-based instructions (1)
{setup.py,setup.cfg,pyproject.toml,**/__init__.py}
📄 CodeRabbit inference engine (AGENTS.md)
Use Python 3.10 or higher as the minimum supported version
Files:
pyproject.toml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build Package
- GitHub Check: test (3.12)
- GitHub Check: test (3.10)
- GitHub Check: test (3.11)
🔇 Additional comments (3)
pyproject.toml (3)
44-60: Dependency consolidation properly addresses issue #420.The core dependencies now include all packages that were previously split across
requirements.txtandsetup.py. The version constraints are well-chosen, and the inline comments improve maintainability. Python 3.10+ minimum version is correctly configured throughout (line 43, tool configs).Based on coding guidelines, Python 3.10+ requirement is satisfied.
66-74: Dev dependencies are appropriate for the project.The additions align well with the project needs:
pytest-asyncio>=0.23.0for async test supportpytest-mock>=3.12.0for test mockingbuild>=0.10.0for the packaging workflowruff>=0.8.0is a valid released version (current release is 0.14.11)The project correctly specifies
requires-python = ">=3.10", meeting the minimum Python version requirement.
48-55: Cannot verify claims about issue #420 or version security status.The dependencies
requests>=2.32.4andcryptography>=42.0.0are present in lines 48–55 of pyproject.toml. However, no references to "issue #420" exist in the repository, and the claim that these additions fix ModuleNotFoundError issues cannot be substantiated. Additionally, verification of current package versions and security advisories was not completed. The pyproject.toml correctly specifiesrequires-python = ">=3.10", meeting the coding guideline requirement.
|
@Anshgrover23 Could you please review this ? |
* refactor: consolidate dependencies into pyproject.toml * refactor: update pip bootstrap commands for development environment * chore: update development dependencies in pyproject.toml
2 participants
This PR consolidates all Python dependencies into
pyproject.tomlas the single source of truth, following modern Python packaging standards (PEP 518/621). It removes deprecatedsetup.pyand legacy requirements files, and updates all documentation and CI workflows to use the new packaging approach.Problems solved
setup.py,requirements.txt, andpyproject.tomlpip install -e .was missing critical dependencies (requests, cryptography, typing-extensions) that were only inrequirements.txtInstallation
Before:
pip install -e . pip install -r requirements.txt pip install -r requirements-dev.txtAfter:
Video.Trimmer.mp4
Related Issues
Fixes #420 - Missing dependencies after
pip install -e .Checklist
pyproject.tomlSummary by CodeRabbit
Chores
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.