Repository navigation
[Release 3.0.0] Restore default camera image data type - #8373
Merged
kellyguo11 merged 1 commit intoOct 8, 2026
Merged
kellyguo11 merged 1 commit into
kellyguo11 merged 1 commit into
Conversation
kellyguo11
requested review from
AntoineRichard,
StafaH,
fatimaanes,
ooctipus and
pbarejko
as code owners
October 7, 2026 21:34
Contributor
|
Contributor
There was a problem hiding this comment.
Isaac Lab Review Bot
All reviewed code categories look good.
- PR description: The title and body match the three-file diff and reported validation; no measurable performance claim or material runtime-cost change requires benchmark evidence.
- Design and architecture: Architecture looks good.
- Scope and over-engineering: No over-engineering issues.
- API: API changes look good.
- Compatibility and deprecation: No breaking changes identified.
- Implementation: Implementation looks good.
- Style consistency: Style consistency looks good.
- Test quality: Test quality looks good.
No blocking issues. No inline findings.
Automated review; human maintainers own approval decisions.
7 of 8 tasks
Contributor
Author
|
run-ci |
maxkra15
pushed a commit
that referenced
this pull request
Oct 14, 2026
# Description Adds the missing `data_type: str = "normals"` parameter to `image_normals` and uses it to select the camera output. The new parameter is keyword-only so the existing positional order for `normalize`, `channel_first`, and `frame_stack` remains compatible. Manager terms resolve callable defaults into `cfg.params` before construction. Unlike the other per-modality image terms, `image_normals` did not declare its data type in the callable signature, so the strict `CameraImageBase` constructor raised `KeyError: 'data_type'`. This change fixes the signature rather than adding a fallback to the base class, keeping default resolution consistent with #8127. Fixes NVBUG 6886551 No new dependencies. ## Type of change - Bug fix (non-breaking change which fixes an issue) ## Release backport - [ ] <!-- backport-active-release --> Backport this pull request to the active release branch after it merges into `develop` The release-branch compatibility fix is already open as #8373, so an automated duplicate backport is not requested. ## Screenshots Not applicable; this fixes observation-term construction. ## Validation - Confirmed the default-resolution regression test fails on the unmodified `develop` branch with `KeyError: 'data_type'`. - Confirmed the positional-compatibility regression test fails before the follow-up fix with `KeyError: False` and passes once `data_type` is keyword-only. - Camera-image unit tests: 26 passed. - `uv run isaaclab -f`: passed, including the changelog gate against `upstream/develop`. - `uv run --isolated --extra dev --extra ov -- make -C docs current-docs`: passed without warnings. ## Checklist - [x] I have read and understood the [contribution guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html) - [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with `uv run isaaclab --format` - [x] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have validated the changed behavior with focused coverage and applied the [test-audit criteria](https://github.com/isaac-sim/IsaacLab/blob/develop/skills/developer/test-audit/SKILL.md) to the test change - [x] I have added a changelog fragment under `source/<pkg>/changelog.d/` for every touched package - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there --------- Co-authored-by: Antoine RICHARD <antoiner@nvidia.com>
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
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.
Description
Restores the
CameraImageBasefallback to a subclass'sdefault_data_typewhen observation term parameters do not includedata_type.image_normalsintentionally reads the fixed"normals"output and does not expose a publicdata_typeargument. The strict parameter lookup introduced in #8127 therefore raisesKeyError: 'data_type'when the observation manager constructs the term. Falling back in the base class keeps the existingimage_normalsAPI unchanged while preserving explicitdata_typeconfiguration for the other image terms.Fixes NVBUG 6886551.
No new dependencies.
Type of change
Release backport
Not applicable: this pull request targets
release/3.0.0directly.Screenshots
Not applicable; this fixes an observation-term construction failure.
Validation
KeyError: 'data_type'.uv run isaaclab -f: all code and style hooks passed.upstream/release/3.0.0tip.Checklist
pre-commitchecks withuv run isaaclab --formatsource/<pkg>/changelog.d/for every touched packageCONTRIBUTORS.mdor my name already exists there