Skip to content

Preserve aliases in lazy_export absolute imports - #7273

Open
sylvesterkaczmarek wants to merge 8 commits into
isaac-sim:developfrom
sylvesterkaczmarek:fix/lazy-export-absolute-aliases
Open

Preserve aliases in lazy_export absolute imports#7273
sylvesterkaczmarek wants to merge 8 commits into
isaac-sim:developfrom
sylvesterkaczmarek:fix/lazy-export-absolute-aliases

Conversation

@sylvesterkaczmarek

Copy link
Copy Markdown

Description

Fixes alias handling for absolute named imports declared in .pyi stubs used by lazy_export().

_parse_stub() currently records only alias.name. For a stub such as:

from math import sqrt as square_root

the runtime export is created as sqrt instead of square_root, so the module does not match the public API described by its stub.

This change records both the source name and the exported name and installs the object under the alias when one is present. Non-aliased absolute imports keep their existing behavior.

Type of change

  • Bug fix

Validation

  • Added a unit regression test using a temporary package with an aliased absolute import in its .pyi stub.
  • The test verifies the alias is callable and appears in __all__, while the original source name is not exported.
  • Added the required isaaclab changelog fragment.

Checklist

  • I have read and understood the contribution guidelines
  • I have added a regression test
  • I have added the required changelog fragment
  • No new dependencies

@sylvesterkaczmarek
sylvesterkaczmarek requested a review from a team August 21, 2026 15:17
@github-actions github-actions Bot added bug Something isn't working isaac-lab Related to Isaac Lab team labels Aug 21, 2026

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Isaac Lab Review Bot

The change correctly preserves aliases for absolute named imports in .pyi stubs by carrying both source and export names through parsing and runtime installation. The regression test verifies the aliased callable and __all__ surface, and the changelog fragment documents the user-visible fix.

  • Design and architecture: The representation change remains confined to the private _parse_stub() helper and its consumer in lazy_export(). Existing package grouping, eager resolution, and wildcard fallback architecture remain unchanged.
  • API: Non-aliased absolute imports retain their existing names and __all__ entries. Aliased imports now expose only the alias, aligning the runtime module surface with the stub-declared public API without changing the deprecated packages argument or wildcard behavior.
  • Implementation: The parser annotation, documentation, collected values, export loop, and __all__ handling were updated consistently. The focused test covers callable resolution and confirms that the source name is not publicly exported. Duplicate aliases across package groups retain the pre-existing package-grouped resolution behavior.

No blocking issues. No inline issue met the actionable-evidence threshold; the assessment above records the review feedback.

Automated review; human maintainers own approval decisions.

@greptile-apps

greptile-apps Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR updates lazy_export() to retain both source and exported names for absolute imports declared with aliases in .pyi stubs.

  • Binds imported objects under their declared aliases and includes those aliases in __all__.
  • Adds a temporary-package regression test and a changelog fragment.
  • Leaves existing direct _parse_stub() tests expecting the old return representation, so the test suite requires coordinated updates.

Confidence Score: 4/5

The alias fix should not merge until the existing _parse_stub() tests are updated for the new tuple representation.

The implementation changes every absolute named import from a string to a tuple, while several existing direct parser assertions still require string lists and will fail when the suite runs.

Files Needing Attention: source/isaaclab/isaaclab/utils/module.py and source/isaaclab_tasks/test/core/test_lazy_export_stubs.py

Important Files Changed

Filename Overview
source/isaaclab/isaaclab/utils/module.py Correctly separates source and export names for runtime binding, but changes _parse_stub()’s directly tested representation without updating existing consumers.
source/isaaclab/test/utils/test_module.py Adds a focused runtime regression test proving that an aliased absolute import is callable and exposed only under its alias.
source/isaaclab/changelog.d/sylvesterkaczmarek-lazy-export-absolute-aliases.patch.rst Accurately documents the alias-preservation fix.

Reviews (1): Last reviewed commit: "Format lazy export changelog fragment" | Re-trigger Greptile

Comment thread source/isaaclab/isaaclab/utils/module.py

@AntoineRichard AntoineRichard left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

AI review disclosure: This review was produced with assistance from multiple AI systems.

Thank you for putting this PR together.

Assessment: Alias parsing and eager export binding preserve as-names without changing unaliased behavior.

Findings: Invalid isaaclab .patch.rst filename and no isaaclab_tasks fragment despite touching its managed tests.

Requested changes: Rename isaaclab fragment to .rst; add an isaaclab_tasks .skip; run changelog gate and focused tests.

Changelog filename: Rename the current <slug>.patch.rst fragment to <slug>.rst for this patch-level change. IsaacLab accepts <slug>.rst (patch), <slug>.minor.rst, and <slug>.major.rst; it does not accept a .patch.rst suffix.

Test scope: Runtime alias/all regression and parser expectations are focused.

Documentation / AI skills: No additional documentation update is required: existing semantics are being repaired. No AI-skill update is required.

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.

2 participants