Skip to content

docs: drop the removed CMake presets from the mujoco_xr example - #996

Open
jiwenc-nv wants to merge 1 commit into
NVIDIA:mainfrom
jiwenc-nv:jiwenc/drop-cmake-preset-references
Open

docs: drop the removed CMake presets from the mujoco_xr example#996
jiwenc-nv wants to merge 1 commit into
NVIDIA:mainfrom
jiwenc-nv:jiwenc/drop-cmake-preset-references

Conversation

@jiwenc-nv

@jiwenc-nv jiwenc-nv commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

CMakePresets.json went away in f8b5eaa, which rewrote the Sphinx build pages but missed examples/mujoco_xr/. Its README still tells readers to run cmake --preset py3.12 and to find the build venv under build/cmake-cpython-312/, and two error messages — the configure-time "mujoco_xr: skipped" and the runtime ABI-mismatch ImportError — still name cmake --preset as the remedy. All now point at cmake -B build.

One behavioural fix rides along. tests/pyproject.toml pinned requires-python = "==3.12.*", which only held because the preset forced 3.12; against the 3.11 default, ctest -L mujoco_xr fails before a test runs. It is now >=3.11,<3.14, matching the example's own pyproject and the camera_viz/tests precedent it cites. The ABI match with _mujoco_xr*.so comes from tests/CMakeLists.txt passing the build interpreter to uv run --python, which the pin never did.

The docs and the version-mismatch FATAL_ERROR also stop suggesting build-py3.12/ — one tree named build/ is enough, since -DISAAC_TELEOP_PYTHON_VERSION already selects the interpreter.

Testing

  • SKIP=check-copyright-year pre-commit run --all-files — passed.
  • make current-docs (sphinx-build -W --keep-going) — build succeeded.
  • Not compiled and ctest not run: no configured build tree on this host.

Summary by CodeRabbit

  • Documentation
    • Updated build instructions to require deleting the existing build directory when changing Python versions.
    • Simplified MuJoCo XR setup commands to use the standard build/ directory and direct CMake commands.
    • Refreshed troubleshooting guidance for Python environment and import errors.
  • Bug Fixes
    • Improved Python-version mismatch messages with clearer recovery steps.
    • Expanded MuJoCo XR test compatibility to Python 3.11–3.13.

Commit f8b5eaa deleted CMakePresets.json and rewrote the build pages, but
examples/mujoco_xr kept telling readers to run `cmake --preset py3.12` and to
look for the build venv under build/cmake-cpython-312 — commands that now fail
outright. Point them at `cmake -B build` instead, in the README and in the two
error messages that name a configure command.

The example's tests pinned requires-python to ==3.12.*, which only held because
the preset always selected 3.12; against the 3.11 default that pin makes
`ctest -L mujoco_xr` fail before it runs. Widen it to the >=3.11,<3.14 range the
rest of the repo uses — tests/CMakeLists.txt already passes the real build
interpreter to `uv run --python`, which is what guarantees the _mujoco_xr*.so
ABI match.

Also drop the -py3.12 suffix from the build directories the docs and the
version-mismatch error suggest: one tree named build/ is enough, and the suffix
tied a directory name to an interpreter that the -D flag already selects.

Signed-off-by: Jiwen Cai <jiwenc@nvidia.com>
@github-actions

Copy link
Copy Markdown
Contributor

📝 Docs preview is not auto-deployed for fork PRs.

A maintainer with write access to NVIDIA/IsaacTeleop can deploy a preview by
commenting /preview-docs on this PR. Once deployed, the preview
will live at:

https://nvidia.github.io/IsaacTeleop/preview/pr-996/

@coderabbitai

coderabbitai Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: a1289571-80f7-410f-9d61-c73f4af42ba3

📥 Commits

Reviewing files that changed from the base of the PR and between 382f213 and 750df46.

📒 Files selected for processing (8)
  • cmake/SetupPython.cmake
  • docs/source/getting_started/build_from_source/index.rst
  • docs/source/references/build.rst
  • examples/mujoco_xr/CMakeLists.txt
  • examples/mujoco_xr/README.md
  • examples/mujoco_xr/python/isaacteleop_examples/mujoco_xr/__init__.py
  • examples/mujoco_xr/tests/pyproject.toml
  • src/python/isaacteleop/rig/launcher.py

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


📝 Walkthrough

Walkthrough

The changes update Python-version build guidance across CMake errors and documentation. Users must delete the existing build/ directory before selecting another Python version. MuJoCo XR instructions now use direct CMake commands and the generic build/ directory instead of presets. MuJoCo XR tests support Python versions from 3.11 through below 3.14. Launcher documentation now references managed wheel build trees.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 750df

The PR updates stale build instructions and aligns the example tests with supported Python versions; no actionable merge-blocking risk remains after normal checks.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately identifies the main change: removing obsolete CMake preset references from the MuJoCo XR example documentation and commands.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2 files. (6 skipped: 6 unsupported.)
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@jiwenc-nv
jiwenc-nv enabled auto-merge (rebase) August 21, 2026 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant