Replace setup.py with uv-compatible pyproject.toml#945
Replace setup.py with uv-compatible pyproject.toml#945kevalmorabia97 wants to merge 2 commits intomainfrom
Conversation
Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com>
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe changes migrate the project's Python package configuration from setup.py to pyproject.toml, the modern Python packaging standard. All project metadata, dependencies, and optional extras are moved to pyproject.toml, CI/CD workflows and configuration files are updated to reference the new configuration file, and the legacy setup.py is removed. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
pyproject.toml (2)
56-63: Consider adding a comment explaining the ONNX runtime version split.The version constraints differ significantly between Python versions (
~=1.22.0for Python ≤3.10 vs~=1.24.2for Python >3.10). A brief inline comment explaining why this split exists would help future maintainers understand the rationale.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pyproject.toml` around lines 56 - 63, Add a short inline comment above the ONNX Runtime dependency block explaining the rationale for the Python-version split (why "~=1.22.0" is used for python_version <= '3.10' and "~=1.24.2" for python_version > '3.10'), referencing the specific package lines "onnxruntime~=1.22.0", "onnxruntime-gpu~=1.22.0", "onnxruntime~=1.24.2", and "onnxruntime-gpu~=1.24.2"; the comment should note compatibility/wheel availability differences across Python versions and platforms (CPU vs GPU, aarch64/Darwin/Windows) so future maintainers understand the constraint choices.
20-21: Consider usingLICENSEinstead ofLICENSE_HEADERforlicense-files.Both
LICENSEandLICENSE_HEADERexist and contain the full Apache 2.0 license text. While the current configuration is valid, the conventional practice is to reference the mainLICENSEfile inlicense-filesrather thanLICENSE_HEADER.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pyproject.toml` around lines 20 - 21, Update the pyproject.toml license-files entry to reference the conventional LICENSE filename instead of LICENSE_HEADER: change the license-files array value from "LICENSE_HEADER" to "LICENSE" so the top-level "license-files" key points to the main license file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@pyproject.toml`:
- Around line 56-63: Add a short inline comment above the ONNX Runtime
dependency block explaining the rationale for the Python-version split (why
"~=1.22.0" is used for python_version <= '3.10' and "~=1.24.2" for
python_version > '3.10'), referencing the specific package lines
"onnxruntime~=1.22.0", "onnxruntime-gpu~=1.22.0", "onnxruntime~=1.24.2", and
"onnxruntime-gpu~=1.24.2"; the comment should note compatibility/wheel
availability differences across Python versions and platforms (CPU vs GPU,
aarch64/Darwin/Windows) so future maintainers understand the constraint choices.
- Around line 20-21: Update the pyproject.toml license-files entry to reference
the conventional LICENSE filename instead of LICENSE_HEADER: change the
license-files array value from "LICENSE_HEADER" to "LICENSE" so the top-level
"license-files" key points to the main license file.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.github/CODEOWNERS.github/workflows/example_tests.yml.github/workflows/gpu_tests.yml.github/workflows/unit_tests.yml.pre-commit-config.yamlpyproject.tomlsetup.pytox.ini
💤 Files with no reviewable changes (2)
- .github/CODEOWNERS
- setup.py
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #945 +/- ##
==========================================
- Coverage 72.15% 72.14% -0.01%
==========================================
Files 210 209 -1
Lines 23515 23511 -4
==========================================
- Hits 16967 16963 -4
Misses 6548 6548 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
9723427 to
4ce4702
Compare
Signed-off-by: Keval Morabia <28916987+kevalmorabia97@users.noreply.github.com>
4ce4702 to
7511ca8
Compare
What does this PR do?
Type of change: Project config update
Remove unnecessary setup.py as the same can be done with pyproject.toml. No change for users in terms of installation command.
Testing
Per PR tests passing is sufficient validation
Summary by CodeRabbit