[Deps] Centralize shared external pins in tools/python_deps.py#5571
[Deps] Centralize shared external pins in tools/python_deps.py#5571hujc7 wants to merge 3 commits into
Conversation
Newton declares `mujoco` and `mujoco-warp` under its `[sim]` extra, so listing them again in IsaacLab's setup.py was redundant and forced the core `isaaclab` package (used by PhysX and Kit backends too) to pull in MuJoCo even when nothing in IsaacLab core imports it. Switch the Newton install spec to `newton[sim] @ git+...` and remove the direct pins. Mirror the same change in the wheel builder manifest so wheel builds stay in sync with `isaaclab_newton/setup.py`.
CI on isaaclab_tasks / isaaclab_newton failed because pip resolves `newton @ git+url` once per URL: the bare spec in isaaclab_visualizers and isaaclab_physx was processed before the `newton[sim] @ git+url` in isaaclab_newton, so the `[sim]` extra (which carries mujoco and mujoco-warp) was dropped and tests hit `ModuleNotFoundError: mujoco`. Switch every `newton @ git+...` site in source/ to `newton[sim] @ ...` so any install order pulls the MuJoCo solver deps transitively.
Six setup.py sites and a wheel-builder TOML each hand-typed the same Newton, Warp, Torch, NumPy, prettytable, and PyOpenGL-accelerate pins. A version bump touched all of them and silently drifted when one site was missed — recent example: only two of six sites switched to `newton[sim] @ git+...`, which let pip resolve the bare `newton` first and drop the requested `[sim]` extra, breaking installs. Move the canonical strings into tools/python_deps.py and have every setup.py import them. The wheel builder's gen_pyproject.py validates that tools/wheel_builder/res/python_packages.toml still matches the constants before emitting the wheel pyproject — drift between Python and TOML now fails the Build Wheel job with a clear message naming the constant and the file out of sync. The dependency graph produced by `pip install` is unchanged from develop; this is pure refactoring.
![isaaclab-review-bot[bot]](https://avatars.githubusercontent.com/u/157846462?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
Thanks for tackling the dependency pin drift problem — this is solid infrastructure work that will pay dividends on future version bumps.
Summary
This PR centralizes shared external dependency pins (Newton, Warp, Torch, NumPy, prettytable, PyOpenGL-accelerate) into a single tools/python_deps.py file, eliminating 6+ redundant version declarations across setup.py files. The wheel builder validation in gen_pyproject.py ensures the TOML stays in sync with the canonical constants.
Findings
1. ✅ Architecture — Clean Single Source of Truth
The design is sound:
- Stdlib-only module imports safely in setup.py before pip resolution
- Constants with docstrings explain why each pin exists (especially the Newton
[sim]extra rationale) - gen_pyproject.py validation catches TOML drift at build time
2. ⚠️ Minor: Path Construction Inconsistency
The setup.py files use two patterns for finding tools/:
# Pattern A (isaaclab, physx, rl, tasks):
sys.path.insert(0, os.path.realpath(os.path.join(EXTENSION_PATH, "..", "..", "tools")))
# Pattern B (newton, visualizers):
_HERE = os.path.dirname(os.path.realpath(__file__))
sys.path.insert(0, os.path.realpath(os.path.join(_HERE, "..", "..", "tools")))Both resolve correctly, but Pattern B is more explicit since _HERE is clearly the setup.py directory. Consider standardizing on Pattern B for clarity.
Severity: Nit / Optional
3. 📝 Pre-existing Drift (As Noted in PR Description)
The PR description already surfaced two pre-existing version mismatches that this PR intentionally leaves out of scope:
gymnasium==1.2.1(setup.py) vsgymnasium==1.2.0(wheel TOML)pin-pink==3.1.0(setup.py) vspin-pink>=2.3.6(wheel TOML)
Tracking for follow-up is appropriate — bundling those fixes here would muddy the refactor.
4. ℹ️ CI Status Note
The Installation Tests failure is unrelated to this PR — it's a Docker buildkit infrastructure issue (resolv.conf.tmp: no such file or directory). The relevant Build Wheel job passed, which exercises the new validation logic.
Verdict
LGTM — clean refactor that solves a real maintenance pain point. The newton[sim] fix is particularly important given how pip's git-URL deduplication can silently drop extras.
1 participant
Summary
Files
Pip behaviour
`pip install` produces the same dependency graph as develop — verified by inspecting each `setup.py`'s install_requires/extras before and after.
Drift in the wheel TOML this PR does NOT auto-fix
While auditing, I noticed two pre-existing drifts between `setup.py` and `tools/wheel_builder/res/python_packages.toml` that are out of scope here:
Surfaced as PR review comments — happy to take a follow-up PR.
Test plan