docs(isaac): backend reference + Phase 1 status banner (5/5 of #31 split)#48
Open
yinsong1986 wants to merge 3 commits into
Open
docs(isaac): backend reference + Phase 1 status banner (5/5 of #31 split)#48yinsong1986 wants to merge 3 commits into
yinsong1986 wants to merge 3 commits into
Conversation
strands-labs#31 split) Part 1 / 5 of the split of strands-labs#31 — tracked by strands-labs#42. Adds the package skeleton for the Isaac Sim backend with zero `omni` overhead on `import strands_robots_sim`: - `pyproject.toml`: - new `[isaac]` extra (lightweight Isaac-companion deps that ARE pip-installable: `usd-core>=24.5,<26.0`, `warp-lang>=1.13.0`). Comment explains that Isaac Sim itself is NOT installable from PyPI — must come via Omniverse Launcher / Isaac Lab / nvcr.io. - new `[all]` aggregate extra (currently delegates to `[isaac]`; will gain `[newton]` etc. as later backends land). - new `[project.entry-points."strands_robots.backends"]` declaring both `isaac` and `isaac_sim` aliases pointing at `strands_robots_sim.isaac.simulation:IsaacSimulation` (the class itself lands in PR-4 / `feat(isaac): IsaacSimulation backend`). - new `pytest>=7.0` in `dev` extras and `[tool.hatch.envs.default]` deps so future PRs in this split chain can run pytest under `hatch run test`. - new `[tool.pytest.ini_options]` with a `gpu` marker so PR-4's GPU-only integ tests can gate on `STRANDS_GPU_TEST=1`. - `strands_robots_sim/isaac/__init__.py`: PEP 562 lazy stub — declares `__all__ = ["IsaacSimulation", "IsaacConfig"]` and routes attribute access through `__getattr__` to deferred imports of `.simulation` and `.config`. The bare `import strands_robots_sim.isaac` does NOT import any `omni` modules; that overhead only fires when a caller actually accesses `IsaacSimulation` or `IsaacConfig`. - `strands_robots_sim/isaac/tests/__init__.py`: empty package marker so future test files in this directory are collectable. Test files themselves land in subsequent slices alongside the code they cover. CI signal: lint clean (black / isort / flake8); the existing `hatch run test` import-smoke passes (it doesn't touch `isaac`, so no missing-submodule errors). Pytest doesn't run yet from `hatch run test` — the test command stays as the import-smoke placeholder in this slice and flips to `pytest strands_robots_sim/isaac/tests/` in PR-3 once the first real test files land. Why this lands first: PR-2 / PR-3 / PR-4 all import from `strands_robots_sim.isaac`, so the package stub + extras + entry points have to exist before any of those files can be code-reviewed against current `main`. Original work by @cagataycali in strands-labs#31 (`413ff15..befb1e3`); this slice cherry-picks just the foundation files. Review-thread anchors on the original commits stay intact for whichever child PR consumes the relevant code.
This was referenced May 26, 2026
cagataycali
previously approved these changes
May 26, 2026
Member
cagataycali
left a comment
cagataycali
left a comment
There was a problem hiding this comment.
Local review on Thor — 3/3 pin tests pass ✅, docs are accurate
Verified locally:
pytest strands_robots_sim/isaac/tests/test_phase1_doc_banner.py -v→ 3 passed- Banner sits between Overview and Installation ✅ (line 17, before
## Installationat line 21) - Banner enumerates the silent-success methods correctly:
add_robot(procedural branch),_load_usd_robot,_load_urdf_robot,add_object,add_camera,replicate✅ - Banner names downstream effects:
get_observation()returns{},render()returns blank frames ✅ render_modeenum in docs matches config.py:headless/rtx_realtime/rtx_pathtracing✅
What's good ✅
- 3-test pin file is well-scoped (file existence, banner-precedes-installation ordering, method enumeration)
- Quick Start sequence matches the actual
IsaacSimulationAPI surface in #47 - Phase 1 status disclosure is upfront and honest — exactly what R2's review feedback asked for
- Install paths cover all three real install vectors (Launcher / Isaac Lab / Docker)
Minor nit
Line 73: render_mode="rtx_realtime" in the Quick Start. Combined with headless=True, this is valid but a bit confusing because "headless" is also a render_mode value. A headless=True, render_mode="rtx_realtime" combo means "run with no GUI but still rasterize for camera renders". Worth a one-liner clarifying that headless controls the viewer, render_mode controls camera-render quality. Not blocking.
Mergeability
✅ Mergeable as-is once #44 lands. No code dependency on #47, so this can land in parallel with PR-4 (as the PR description correctly states).
9 tasks
…abs#43 (PR strands-labs#44 R1) Closes @cagataycali's R1 review on strands-labs#44 -- the lazy-import contract ships with concrete CI signal now, not just a manual probe in the PR description. Three coordinated changes per the review: 1. **Port `tests/test_entrypoint.py` (140 LOC, 10 tests)** from the closed orphan PR strands-labs#43 (`feat/isaac-extras-and-lazy-import`). These tests verify *only* PR-1's surface and have **zero dependency on `simulation.py`** (which lands in PR-4 of the strands-labs#31 split): - `TestEntryPointDeclaration` (6 tests): - `test_pyproject_exists` - `test_isaac_entry_point_declared_in_pyproject` - `test_isaac_sim_alias_entry_point_declared_in_pyproject` - `test_isaac_extra_declared_in_pyproject` - `test_isaac_extra_includes_pytest` - `test_entry_points_visible_via_importlib_metadata_when_installed` (skips with actionable hint when not pip-installed) - `TestLazyImportSurface` (4 tests): - `test_import_isaac_does_not_load_omni` -- pins the `omni`-not- loaded contract via `sys.modules` set diff before/after import. - `test_isaac_subpackage_exposes_lazy_attrs_in___all__` - `test_unknown_attr_raises_attributeerror` -- pins the PEP 562 `__getattr__` raises `AttributeError` (not `ImportError`) on unknown attrs. - `test_dunder_getattr_is_present` 2. **Add `pytest>=7.0` to the `[isaac]` extra** in `pyproject.toml`. Means `pip install '.[isaac]'` covers the test deps without a separate dev extra on CI hosts. Also pinned by `test_isaac_extra_includes_pytest`. 3. **Flip `hatch run test`** from the import-smoke placeholder (`python -c "import strands_robots_sim; ..."`) to `pytest strands_robots_sim/isaac/tests/ -v`. The 10 tests collect and pass standalone; PR-4's simulation-module tests join the collection automatically when that PR rebases. Verification: - `pytest strands_robots_sim/isaac/tests/ -v` -> **10 passed in 0.14s** - `black --check` / `isort --check-only` / `flake8` (max-line=120) on `strands_robots_sim/` + `examples/` -> clean - AST parse on touched files -> ok - The 10 tests have no import path through `simulation.py`; they validate only PR-1's actual surface (pyproject parsing, entry- point declaration, PEP 562 `__getattr__` contract). The original `test_entrypoint.py` on `cagataycali/feat/isaac-sim` imports from `strands_robots_sim.isaac.simulation`, which is why PR-1 of the split couldn't ship that file unmodified. The version ported here was rewritten on the strands-labs#43 branch specifically for the PR-1 surface; cleanly self-contained against this slice.
…s-labs#31 split) Part 5 / 5 of the split of strands-labs#31 — tracked by strands-labs#42. Branched off PR-1 (`pr1/isaac-foundation` -> strands-labs#44); rebases cleanly onto `main` once PR-1 merges. Parallel-mergeable with PR-4 (strands-labs#47, simulation) -- this slice has no code dependency on simulation.py. Adds the user-facing reference doc for the Isaac backend plus the companion test file that pins R2's "Phase 1 status" disclosure banner: - `docs/backends/isaac.md` (208 LOC): Install + Quick Start + API reference + environment variables + R2's Phase 1 status banner before the Installation section. The banner discloses that the Phase 1 skeleton silently no-ops the data plane (`add_robot` procedural branch, `_load_usd_robot`, `_load_urdf_robot`, `add_object`, `add_camera`, `replicate` all return `status: "success"` without instantiating the underlying USD prim or articulation handle, with the observable downstream effect that `get_observation()` returns `{}` and `render()` returns blank frames). R2 reviewer asked for this explicitly so a user following the Quick Start sees the disclaimer before the silent path. - `strands_robots_sim/isaac/tests/test_phase1_doc_banner.py` (~75 LOC, 3 tests): Carved out of cagataycali's original `test_phase1_doc_honesty.py` -- the `TestIsaacDocsPhase1Banner` class only. The companion `TestG1DOFCount` class lives in PR-3 (strands-labs#46) alongside `procedural.py`. Pin tests: - `test_isaac_docs_file_exists`: truth-source pin for the doc file's location. - `test_phase1_banner_present_before_installation`: pins that "Phase 1 status" appears AND precedes the `## Installation` heading. - `test_phase1_banner_names_the_silent_methods`: pins that the banner enumerates `add_robot`, `replicate`, `get_observation` by name (so a future maintainer who reads only the banner knows which API surfaces are affected). CI signal: lint clean (black / isort / flake8); the 3 doc-banner pin tests pass standalone (`pytest test_phase1_doc_banner.py -v` -> 3 passed). Why split `test_phase1_doc_honesty.py` into G1-DOF (PR-3) + doc- banner (this PR): keeps each parallel-mergeable slice CI-green standalone. Shipping the original combined file in either PR would couple them (PR-3 would CI-red until PR-5 lands, or vice versa). The combined coverage equals the original file. Original work by @cagataycali in strands-labs#31 (`413ff15..85e180f`); this slice cherry-picks `docs/backends/isaac.md` plus the carved-out banner test file. R2's banner addition (commit `85e180f`) is already baked into this doc.
yinsong1986
force-pushed
the
pr5/isaac-docs
branch
from
9c18e83 to
e811fec
Compare
Contributor
Author
|
Rebased onto SHA shift on this branch:
Same content; rebased commit object only. No parallel-session activity on this branch. Local verification post-rebase: PR is approved by @cagataycali and |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Part 5 / 5 of the split of #31 — tracked by
#42.Adds the user-facing reference doc for the Isaac backend plus the companion test file that pins R2's "Phase 1 status" disclosure banner. Branched off PR-1 (#44); rebases cleanly onto
mainonce PR-1 merges. Parallel-mergeable with PR-4 (#47) — no code dependency onsimulation.py.What's in this slice
docs/backends/isaac.mdstrands_robots_sim/isaac/tests/test_phase1_doc_banner.pytest_phase1_doc_honesty.py::TestIsaacDocsPhase1Banner; pins the banner content + orderingR2 review-fix carry-over
R2 (commit
85e180fon #31) added a "Phase 1 status" banner todocs/backends/isaac.mdbetween Overview and Installation. The banner discloses that the Phase 1 skeleton silently no-ops the data plane:add_robot(procedural branch)_load_usd_robot/_load_urdf_robotadd_objectadd_camerareplicate…all return
status: "success"without instantiating the underlying USD prim or articulation handle. The observable downstream effect:get_observation()returns{}andrender()returns blank frames.Reviewer asked for this explicitly so a user following the Quick Start sees the disclaimer before following the documented call sequence — the banner sits before the
## Installationheading.Why split
test_phase1_doc_honesty.pyThe original test file mixed two concerns:
TestG1DOFCount(3 tests)tests/test_procedural_g1_dof.pyTestIsaacDocsPhase1Banner(3 tests)tests/test_phase1_doc_banner.pySplitting keeps each parallel-mergeable slice CI-green standalone. Shipping the original combined file in either PR would couple them (PR-3 would CI-red until PR-5 lands, or vice versa). Combined coverage equals the original file.
CI signal
hatch run lint(black --check/isort --check-only/flake8overstrands_robots_sim/+examples/) — clean.pytest strands_robots_sim/isaac/tests/test_phase1_doc_banner.py -v→ 3 passed.## Installationheading, (c) the banner enumeratesadd_robot,replicate,get_observationby name.Dependency chain
PR-1 (#44, foundation) → PR-5 (this), parallel with PR-4 (#47, simulation). This is the closing slice — when PR-5 lands and #31 is closed, the umbrella tracker #42 closes too.
Diff: 2 files, +288 / 0 LOC.
Original authorship
Original work by @cagataycali in #31 (
413ff15..85e180f). This slice cherry-picksdocs/backends/isaac.mdplus the carved-out banner test file. R2's banner addition (commit85e180f) is already baked into this doc.