Skip to content

Defer eager submodule imports via lazy .pyi stubs#5932

Open
ooctipus wants to merge 2 commits into
isaac-sim:developfrom
ooctipus:fix/fix_lazy_export
Open

Defer eager submodule imports via lazy .pyi stubs#5932
ooctipus wants to merge 2 commits into
isaac-sim:developfrom
ooctipus:fix/fix_lazy_export

Conversation

@ooctipus
Copy link
Copy Markdown
Collaborator

@ooctipus ooctipus commented Jun 3, 2026

Summary

  • lazy_export() packages declare their export surface in a sibling .pyi stub, and relative submodule imports (from . import <sub>) belong in that stub so they resolve lazily. A few __init__.py files still imported submodules eagerly, which forced the submodule (and its transitive imports) to load at package-import time and defeated the lazy stub.
  • isaaclab/envs/__init__.py: moved from . import mdp, ui into envs/__init__.pyi (added to the stub imports and __all__).
  • isaaclab/sensors/ray_caster/__init__.py: removed the redundant from . import patterns — the stub already declared patterns.

After the change, import isaaclab.envs no longer pulls mdp/ui eagerly (verified they are absent from sys.modules until first access), while isaaclab.envs.mdp, from isaaclab.envs import mdp, ui, ray_caster.patterns, and all named class exports still resolve correctly.

Left intentionally unchanged: isaaclab/utils/warp/__init__.py eagerly runs wp.init() and monkeypatches wp.to_torch (its ProxyArray import feeds that runtime shim), so it is genuine eager initialization rather than a re-export.

Test plan

  • import isaaclab.envsmdp/ui not in sys.modules; resolve on access.
  • from isaaclab.envs import mdp, ManagerBasedRLEnv and import isaaclab.sensors.ray_caster; ray_caster.patterns all resolve.
  • pre-commit (ruff / ruff-format) passes on the changed files.

@ooctipus ooctipus requested a review from pascal-roth as a code owner June 3, 2026 00:44
@github-actions github-actions Bot added bug Something isn't working isaac-lab Related to Isaac Lab team labels Jun 3, 2026
Copy link
Copy Markdown

@isaaclab-review-bot isaaclab-review-bot Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI Review Summary

PR Title: Defer eager submodule imports via lazy .pyi stubs

Overview

This PR correctly fixes the lazy export pattern by moving eager submodule imports from __init__.py files into their corresponding .pyi stub files. This ensures that lazy_export() works as intended - submodules are only loaded when actually accessed, not at package import time.

Changes Analysis

1. isaaclab/envs/__init__.py__init__.pyi

Correct approach: Moved from . import mdp, ui from the .py file to the .pyi stub, and added "mdp" and "ui" to __all__. This ensures these submodules are declared in the type stub for IDE support while remaining lazily loaded at runtime.

2. isaaclab/sensors/ray_caster/__init__.py

Correct approach: Removed the redundant from . import patterns since the stub already declares patterns in its exports. This was causing patterns to be eagerly loaded, defeating the lazy export mechanism.

3. changelog.d/fix-fix_lazy_export.skip

Appropriate: Using a .skip file for internal import hygiene changes that have no user-facing impact.

Technical Correctness

The lazy export pattern in IsaacLab relies on:

  1. The .pyi stub declaring the public interface (for type checkers/IDEs)
  2. The lazy_export() call setting up __getattr__ to lazily import on first access
  3. No eager imports in the __init__.py that would load submodules before they're needed

This PR correctly ensures condition #3 is met.

Verification

The author's test plan is appropriate - verifying that import isaaclab.envs no longer loads mdp/ui into sys.modules until accessed, while still allowing explicit imports like from isaaclab.envs import mdp to work.

Suggestions

  1. Minor: Consider adding a comment in the code or contributing guide noting that submodule re-exports should go in .pyi stubs rather than __init__.py to prevent future regressions.

  2. Question: The PR description mentions isaaclab/utils/warp/__init__.py was intentionally left unchanged. It would be helpful to add a brief inline comment there explaining the eager initialization is intentional, to prevent someone from "fixing" it later.


This is an automated review. A human reviewer should verify the changes align with the project's lazy loading architecture.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Jun 3, 2026

Greptile Summary

This PR fixes eager submodule loading in two packages by moving the relevant from . import … lines out of the __init__.py files and into their sibling .pyi stubs, where lazy_export() already reads them and registers each name as a lazily-resolved attribute.

  • isaaclab/envs: from . import mdp, ui removed from __init__.py and added to __init__.pyi (along with "mdp" and "ui" in __all__), so those submodules no longer load at import isaaclab.envs time.
  • isaaclab/sensors/ray_caster: from . import patterns removed from __init__.py; patterns was already declared in __init__.pyi, making the eager import redundant.
  • changelog.d/: A .skip marker is added to signal that no user-facing changelog entry is needed for this internal cleanup.

Confidence Score: 5/5

Safe to merge — changes are limited to moving two from . import lines from eager __init__.py into lazy .pyi stubs, with no logic or public API modified.

The diff is narrow: two eager import lines removed, one .pyi stub updated to include them with matching __all__ entries, and one redundant import removed. The _parse_stub / lazy_export machinery already handles from . import X in stubs (level-1 non-star imports go directly into the filtered body for lazy_loader), so the lazy semantics are preserved correctly. No behavior visible to downstream users changes — submodule access still resolves correctly, just deferred until first use.

No files require special attention.

Important Files Changed

Filename Overview
source/isaaclab/isaaclab/envs/init.py Removes eager from . import mdp, ui — now handled lazily via the sibling .pyi stub.
source/isaaclab/isaaclab/envs/init.pyi Adds "mdp" and "ui" to __all__ and adds from . import mdp, ui so lazy_export() picks them up as lazily-resolved submodules.
source/isaaclab/isaaclab/sensors/ray_caster/init.py Removes redundant from . import patterns that was already declared in __init__.pyi, eliminating the eager load.
source/isaaclab/changelog.d/fix-fix_lazy_export.skip Changelog skip marker; correctly signals this is an internal import-hygiene fix with no user-facing entry needed.

Sequence Diagram

sequenceDiagram
    participant U as User code
    participant E as isaaclab.envs
    participant LE as lazy_export()
    participant PS as _parse_stub(__init__.pyi)
    participant LL as lazy_loader.attach_stub

    U->>E: import isaaclab.envs
    E->>LE: lazy_export()
    LE->>PS: parse envs/__init__.pyi
    PS-->>LE: "filtered_body has from . import mdp, ui (needs_filter=False)"
    LE->>LL: attach_stub(package_name, caller_file)
    LL-->>LE: __getattr__, __dir__, __all__
    LE-->>E: sets mod.__getattr__, mod.__all__
    E-->>U: module ready (mdp/ui NOT yet loaded)

    U->>E: isaaclab.envs.mdp (first access)
    E->>LL: __getattr__("mdp")
    LL-->>E: import isaaclab.envs.mdp (lazy)
    E-->>U: mdp submodule
Loading

Reviews (1): Last reviewed commit: "Add changelog skip fragment for lazy-exp..." | Re-trigger Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working isaac-lab Related to Isaac Lab team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant