chore: modernize build system — uv lockfile, Python >=3.12#83
chore: modernize build system — uv lockfile, Python >=3.12#83cagataycali wants to merge 5 commits intostrands-labs:mainfrom
Conversation
|
|
||
| [project.urls] | ||
| Homepage = "https://github.com/strands-labs/robots" | ||
| Documentation = "https://github.com/strands-labs/robots#readme" |
There was a problem hiding this comment.
why were some project URLs removed?
There was a problem hiding this comment.
Fixed in b462e0c — restored Documentation and Changelog URLs.
🤖 AI agent response. Strands Agents. Feedback welcome!
There was a problem hiding this comment.
Restored in b462e0c — all original project URLs are back. Diff between main and this branch for [project.urls] is now zero (Homepage, Documentation, Repository, Issues, Changelog all present and identical).
🤖 AI agent response. Strands Agents. Feedback welcome!
|
|
||
| [tool.hatch.envs.default.scripts] | ||
| test = "pytest {args:tests}" | ||
| test-integ = "pytest tests_integ/ -v --timeout=300 {args}" |
There was a problem hiding this comment.
I found the helper scripts useful. Now to run tests I have to remember these args. For lint I have to know we're using ruff, etc.
There was a problem hiding this comment.
Fixed in b462e0c — restored all hatch helper scripts (test, test-integ, lint, format) and the [tool.hatch.envs.default] dependencies block. CI uses uv run but the hatch scripts remain as the developer interface.
🤖 AI agent response. Strands Agents. Feedback welcome!
There was a problem hiding this comment.
Is that ideal? How do developers know that what they build will pass CI if they're not running the same commands?
- Bump requires-python from >=3.10 to >=3.12 - Move opencv-python-headless and Pillow to [sim] extra - Add [sim] extra: mujoco, robot_descriptions, opencv, Pillow - Add .python-version (3.12) - Add uv.lock for deterministic builds - Switch CI from hatch to uv - Add libosmesa6-dev to CI for MuJoCo headless rendering - Remove hatch env scripts (use uv run directly) - Remove --cov from pytest addopts (run coverage explicitly) - Update ruff target-version and mypy python_version to 3.12 - Trim pyproject.toml comments for readability Why Python 3.12+: match statements, type aliases, ExceptionGroup patterns used in upcoming simulation code. MuJoCo 3.x works best with 3.12+. No users have reported needing 3.10/3.11 support. Why move opencv/Pillow to [sim]: core strands-robots (hardware control only) shouldn't require heavy image processing deps.
b462e0c to
8b7a6b7
Compare
yinsong1986
left a comment
There was a problem hiding this comment.
All review comments addressed. LGTM.
Status Update — Review feedback verificationI verified the branch state against all review threads. Summary: awsarron's feedback (9 threads) — all addressed in current HEAD (
|
| Concern | Status |
|---|---|
| Keywords removed (gr00t, etc.) | ✅ Present — matches main |
[sim] extra / MuJoCo scope |
✅ Removed — deferred to MuJoCo PR |
| Project URLs removed | ✅ Present — matches main |
| Hatch helper scripts removed | ✅ Present — test, test-integ, lint, format all intact |
| Dev deps installation path | ✅ [dev] extra intact, hatch.envs.default.dependencies present |
| Comments trimmed (×2) | ✅ Comment count matches main (14 each) |
yinsong1986's feedback (2 threads) — responded but not code-changed
- Python 3.12 breaking change: Author confirmed intentional (LeRobot >=0.5.0 requires 3.12). PR description should include a "Breaking Changes" callout — this has NOT been added yet.
--covremoval: Author confirmed intentional. AGENTS.md doc update promised — also NOT pushed yet.
Net delta from main
The actual changes in this PR are now narrowly scoped:
requires-python:>=3.10→>=3.12- Classifiers: removed
3.10and3.11entries target-version/python_version:py310/3.10→py312/3.12addopts: removed--cov=strands_robots --cov-report=term-missing- CI:
hatch→uv(workflow updated) - Added
.python-version(3.12) anduv.lock
Needs before merge
- ⬜ awsarron re-review (fixes are in code but threads are unresolved)
- ⬜ PR description updated with "Breaking Changes" section
- ⬜ AGENTS.md coverage command documentation
🤖 AI agent response. Strands Agents. Feedback welcome!
- Add uv sync as recommended install path alongside pip - Document uv run pytest commands as alternative to hatch scripts - Add explicit coverage command (removed from default pytest addopts) - Update Python version convention from 3.10+ to 3.12+ Addresses review feedback from yinsong1986 on documenting the --cov removal and the Python version change.
📝 Unresolved Thread StatusQuick audit of the 9 unresolved review threads: @awsarron feedback (7 threads)
@yinsong1986 feedback (2 threads)
Action needed
🤖 AI agent response. Strands Agents. Feedback welcome! |
Addresses review comment — hatch lint/format scripts now run 'ruff check .' and 'ruff format .' (whole repo) matching CI exactly. AGENTS.md updated to lead with uv commands (same as CI) and demote hatch to documented aliases.
- CI lint: ruff check/format scoped to strands_robots tests tests_integ - CI tests: restore --cov=strands_robots --cov-report=term-missing - pyproject.toml addopts: restore coverage flags - Hatch scripts: scoped to source dirs (matching CI exactly) - AGENTS.md: all commands now identical to CI
Address awsarron's review feedback: developers should run the same commands locally as CI to prevent pass-locally-fail-CI surprises. Changes: - hatch test script now matches CI: -x --strict-markers --cov - Remove mypy from lint script (not in CI workflow) - Add comment linking to test-lint.yml for traceability
TL;DR
Modernize the build system: add uv lockfile, bump Python to ≥3.12. No code logic changes. No dependency changes.
requires-pythonbumped from>=3.10to>=3.12. Users on Python 3.10 or 3.11 must upgrade. This is required because LeRobot >=0.5.0 (a core dependency) requires Python 3.12+.--covremoved frompytest.ini_options.addopts. Coverage can be run explicitly:pytest-covduring development.What changed
pyproject.tomlrequires-python≥3.12, ruff/mypy target py312, remove 3.10/3.11 classifiers, remove--covfrom default addoptsAGENTS.md.python-version3.12.github/workflows/test-lint.yml.gitignoreuv.lockWhy Python ≥3.12
pip install strands-robots[lerobot]silently fails on 3.10/3.11matchstatements,typealiases,ExceptionGrouppatternsWhy uv
uv.lock(reproducible CI)hatch run test,hatch run lint,hatch run formatNOT in this PR
[sim]extra (→ separate PR when mujoco code lands)Testing
uv sync --extra all --extra dev && uv run pytest tests/— all tests passuv run ruff check . && uv run ruff format --check .— cleanpip install -e ".[all,dev]" && pytest— backward compat🤖 AI agent response. Strands Agents. Feedback welcome!