Skip to content

Migrated golden images to Git LFS#5361

Merged
huidongc merged 2 commits into
isaac-sim:developfrom
huidongc:png-to-use-lfs
Apr 25, 2026
Merged

Migrated golden images to Git LFS#5361
huidongc merged 2 commits into
isaac-sim:developfrom
huidongc:png-to-use-lfs

Conversation

@huidongc
Copy link
Copy Markdown
Collaborator

@huidongc huidongc commented Apr 23, 2026

Description

Migrated golden images source/isaaclab_tasks/test/golden_images/**/*.png to Git LFS

Fixes # (issue)

Type of change

  • Test-only change

Checklist

  • I have read and understood the contribution guidelines
  • I have run the pre-commit checks with ./isaaclab.sh --format
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the changelog and the corresponding version in the extension's config/extension.toml file
  • I have added my name to the CONTRIBUTORS.md or my name already exists there

@github-actions github-actions Bot added the isaac-lab Related to Isaac Lab team label Apr 23, 2026
@huidongc huidongc changed the title Migrated golden images source/isaaclab_tasks/test/golden_images/**/*.png to Git LFS Migrated golden images to Git LFS Apr 23, 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.

🤖 Isaac Lab Review Bot

Summary

This PR migrates 78 PNG golden images used for rendering correctness tests from regular Git storage to Git LFS (Large File Storage). The implementation is correct: it adds the appropriate .gitattributes pattern, bumps the extension version, and documents the change in the changelog. All PNG files in the golden_images directory now show LFS pointer content instead of binary data.

Architecture Impact

Self-contained. This is a repository infrastructure change that affects only how Git stores these binary test assets. No code logic, imports, or runtime behavior is modified. The tests that consume these golden images (test_rendering_correctness.py based on the changelog) will continue to work identically—Git LFS is transparent to file consumers once the repository is properly cloned with LFS support.

Implementation Verdict

Ship it — This is a straightforward, well-executed infrastructure improvement.

Test Coverage

Not applicable for this change type. This PR modifies test assets (golden images), not test logic. The existing test_rendering_correctness.py tests will validate these images continue to work correctly. No new tests are needed for a Git LFS migration.

CI Status

No CI checks available yet. Once CI runs, verify:

  1. Any jobs that clone the repo have git-lfs installed and run git lfs pull
  2. Rendering correctness tests pass (images are fetched and compared correctly)

Findings

🔵 Improvement: .gitattributes:14 — Consider adding a generic *.png rule for future-proofing

The current pattern source/isaaclab_tasks/test/golden_images/**/*.png is very specific. If golden images are added elsewhere in the repo, they won't automatically use LFS. Consider whether a broader *.png filter=lfs diff=lfs merge=lfs -text rule (or at minimum documenting this constraint) would be beneficial. However, being conservative here is also reasonable to avoid pulling unrelated small PNGs into LFS.

🔵 Improvement: CHANGELOG.rst:4 — Future date in changelog

The changelog entry shows 1.5.24 (2026-04-23) — this date is in the future (2026). While this appears consistent with other entries in this file (suggesting the project uses future dates as a convention or the system clock context is different), verify this is intentional and matches project release conventions.

🔵 Improvement: Documentation — Consider adding contributor guidance

For contributors who will add new golden images, it would be helpful to document in a CONTRIBUTING guide or README that:

  1. New golden images under source/isaaclab_tasks/test/golden_images/ are automatically tracked by LFS
  2. Contributors need git-lfs installed to properly commit/fetch these files

No critical or warning-level issues found. The LFS migration is implemented correctly.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 23, 2026

Greptile Summary

This PR migrates 79 test golden images under source/isaaclab_tasks/test/golden_images/**/*.png from plain Git objects to Git LFS, adds the corresponding .gitattributes rule, and bumps the extension version to 1.5.24 with a changelog entry. No logic or test code was changed.

Confidence Score: 5/5

Safe to merge — pure LFS migration with no logic changes.

All changes are infrastructural (LFS tracking rule, version bump, changelog). No code logic was modified and there are no P0/P1 findings.

No files require special attention.

Important Files Changed

Filename Overview
.gitattributes Added LFS tracking rule for source/isaaclab_tasks/test/golden_images/**/*.png; pattern correctly scoped to the golden-images subdirectory only.
source/isaaclab_tasks/config/extension.toml Patch version bump from 1.5.23 → 1.5.24, consistent with the changelog entry.
source/isaaclab_tasks/docs/CHANGELOG.rst New 1.5.24 changelog entry documents the LFS migration; formatting and date are correct.
source/isaaclab_tasks/test/golden_images/cartpole/physx-isaacsim_rtx_renderer-rgb.png Representative golden image re-committed as an LFS pointer; all 79 PNG files in the golden_images tree follow the same pattern.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[git commit / push PNG] --> B{.gitattributes\nsource/isaaclab_tasks/test/\ngolden_images/**/*.png}
    B -->|filter=lfs| C[Git LFS\npointer stored\nin git tree]
    B -->|diff=lfs merge=lfs -text| D[Full binary\nstored in\nLFS server]
    C --> E[Checkout: pointer\nreplaced with\nactual PNG]
    D --> E
Loading

Reviews (1): Last reviewed commit: "remove unnecessary change" | Re-trigger Greptile

@huidongc huidongc requested a review from pbarejko April 23, 2026 02:04
Path-scope LFS to source/isaaclab_tasks/test/golden_images/**/*.png with
diff=lfs/merge=lfs so git diff can use materialized bytes. Renormalize
tracked golden baselines to pointers.

Bump isaaclab_tasks to 1.5.25.
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.

🤖 Isaac Lab Review Bot

Summary

This PR migrates 78 golden image PNG files to Git LFS storage by adding a path-based LFS rule in .gitattributes. The change is correctly implemented — the glob pattern source/isaaclab_tasks/test/golden_images/**/*.png matches all existing golden images, and the version/changelog updates are appropriate.

Architecture Impact

Self-contained. This is a repository configuration change that affects only how Git stores and retrieves these test fixture files. No code paths are modified — tests that consume these golden images will continue to work identically since LFS is transparent to file reads.

Implementation Verdict

Ship it — The implementation is correct and follows established patterns in the repository.

Test Coverage

This is a test infrastructure change, not a code change. The golden images themselves are test fixtures used by test_rendering_correctness.py. No additional tests are needed for this migration — if LFS were misconfigured, existing rendering correctness tests would fail due to receiving LFS pointer files instead of actual image data.

CI Status

  • pre-commit: success — LFS tracking rule is syntactically valid
  • Build Wheel: success — Package builds correctly
  • ⏳ Multiple jobs pending — Full CI validation not yet complete, but no blockers expected for this infra change

Findings

🔵 Improvement: .gitattributes:14 — Consider adding a generic *.png rule for broader coverage

The current rule is highly specific:

source/isaaclab_tasks/test/golden_images/**/*.png filter=lfs diff=lfs merge=lfs -text

If golden images are added elsewhere in the future (e.g., other test directories), they won't be tracked by LFS. However, this specificity may be intentional to avoid LFS overhead for documentation images. No action required — just a consideration for future golden image locations.

🔵 Improvement: CHANGELOG.rst:4 — Future-dated changelog entry

The changelog shows 1.5.25 (2026-04-24) which appears to be a placeholder date in the future (2026). This is likely intentional for the development branch's release workflow, but worth confirming this matches your versioning conventions.


The PR correctly:

  1. Adds the LFS filter rule with proper attributes (filter=lfs diff=lfs merge=lfs -text)
  2. Places it logically before the line-ending rules in .gitattributes
  3. Bumps the version in extension.toml (1.5.24 → 1.5.25)
  4. Documents the change in CHANGELOG.rst
  5. All 78 PNG files now show as LFS pointer files (visible in the full file contents showing version https://git-lfs.github.com/spec/v1 headers)

@huidongc huidongc merged commit 1d916c3 into isaac-sim:develop Apr 25, 2026
31 checks passed
@huidongc huidongc deleted the png-to-use-lfs branch April 25, 2026 08:21
matthewtrepte pushed a commit to matthewtrepte/IsaacLab that referenced this pull request Apr 26, 2026
# Description

Migrated golden images
``source/isaaclab_tasks/test/golden_images/**/*.png`` to Git LFS

Fixes # (issue)

<!-- As a practice, it is recommended to open an issue to have
discussions on the proposed pull request.
This makes it easier for the community to keep track of what is being
developed or added, and if a given feature
is demanded by more than one party. -->

## Type of change

- Test-only change

## 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
`./isaaclab.sh --format`
- [x] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works
- [x] I have updated the changelog and the corresponding version in the
extension's `config/extension.toml` file
- [x] I have added my name to the `CONTRIBUTORS.md` or my name already
exists there
mmichelis pushed a commit to mmichelis/IsaacLab that referenced this pull request Apr 29, 2026
# Description

Migrated golden images
``source/isaaclab_tasks/test/golden_images/**/*.png`` to Git LFS

Fixes # (issue)

<!-- As a practice, it is recommended to open an issue to have
discussions on the proposed pull request.
This makes it easier for the community to keep track of what is being
developed or added, and if a given feature
is demanded by more than one party. -->

## Type of change

- Test-only change

## 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
`./isaaclab.sh --format`
- [x] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works
- [x] I have updated the changelog and the corresponding version in the
extension's `config/extension.toml` file
- [x] I have added my name to the `CONTRIBUTORS.md` or my name already
exists there
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

isaac-lab Related to Isaac Lab team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants