Repository navigation
Use setup-uv defaults for Python and the venv - #59
Conversation
|
👋 Hello @glenn-jocher, thank you for submitting a
For more guidance, please refer to our Contributing Guide. Don't hesitate to leave a comment if you have any questions. Thank you for contributing to Ultralytics! 🚀 |
UltralyticsAssistant
left a comment
There was a problem hiding this comment.
🔍 PR Review
Made with ❤️ by Ultralytics Actions
Reviewed the CI workflow’s Python setup and generated-package installation changes. The venv setup and removal of --system are consistent; no concrete regressions found. LGTM.
UltralyticsAssistant
left a comment
There was a problem hiding this comment.
🔍 PR Review 2
Made with ❤️ by Ultralytics Actions
Since Review 1, the diff is unchanged. I’m reversing the prior LGTM based on the current status of upstream #915: it remains open, but this workflow references @main rather than the branch intended for validation. The rest of the Python setup and install changes look consistent.
💬 Posted 1 inline comment
- 💡 MEDIUM
.github/workflows/ci.yml:35While ultralytics/actions#915 is still open, this usesmaininstead of thesetup-uv-defaultsbranch specified for validating the new defaults. CI therefore won't exercise the action revision this migration depends on. Point to@setup-uv-defaultsfor validation, then switch to@mainafter #915 merges as planned.
| bun-version: latest | ||
|
|
||
| - uses: actions/setup-python@v7 | ||
| - uses: ultralytics/actions/setup-uv@main |
There was a problem hiding this comment.
ultralytics/actions#915 merged at 17:18 UTC (da5463a, released as 0.4.0), so setup-uv@main now carries the new defaults and this CI run exercises them. The validation commit pointing at @setup-uv-defaults was already reverted, and the PR description is updated to match.
| bun-version: latest | ||
|
|
||
| - uses: actions/setup-python@v7 | ||
| - uses: ultralytics/actions/setup-uv@main |
There was a problem hiding this comment.
ultralytics/actions#915 merged at 17:18 UTC (da5463a, released as 0.4.0), so setup-uv@main now carries the new defaults and this CI run exercises them. The validation commit pointing at @setup-uv-defaults was already reverted, and the PR description is updated to match.
UltralyticsAssistant
left a comment
There was a problem hiding this comment.
🔍 PR Review 3
Made with ❤️ by Ultralytics Actions
Since Review 2, the upstream action change has merged, resolving the prior concern about using @main. The CI workflow’s Python setup and SDK installation changes are consistent with the described defaults; no concrete regressions found. LGTM.
|
🎉 This PR was merged—thank you, @glenn-jocher! CI now installs generated SDK dependencies into the
|
Replaces
actions/setup-pythonwithultralytics/actions/setup-uv. By default, that action now activates a venv on the newest Python preinstalled on the runner, with no Python download. Packages install into that venv, so--systemis dropped. Jobs that pin a Python version keep the pin onsetup-uv, which uses the preinstalled copy when the runner has one.CI first ran against the
setup-uv-defaultsbranch of ultralytics/actions#915. #915 is now merged (da5463a, released as 0.4.0), sosetup-uvis back on@main, which carries the new defaults.🛠️ PR Summary
Made with ❤️ by Ultralytics Actions
🌟 Summary
CI now uses
ultralytics/actions/setup-uvto set up Python and its virtual environment, so generated SDK dependencies install into that environment instead of system Python.📊 Key Changes
actions/setup-pythonand the separatesetup-uvstep with onesetup-uvstep, retaining the Python version from the CI matrix.--systemfrom the generated Python SDK installation command.🎯 Purpose & Impact
setup-uvenvironment for SDK checks; there is no user-facing change to the generated SDK.