Fix #8442: pass meta_data to EnsureChannelFirst when track_meta is False#8835
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughLoadImage.call now passes the loaded image metadata to EnsureChannelFirst by calling EnsureChannelFirst()(img, meta_dict=meta_data) when ensure_channel_first=True. Tests were updated to import get_track_meta, and a new test (test_track_meta_false_ensure_channel_first) temporarily disables global metadata tracking, runs LoadImage(image_only=True, ensure_channel_first=True) on a prepared NIfTI, asserts the output is a plain torch.Tensor with shape (1, 128, 128, 128), and restores the previous tracking state. Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@monai/transforms/io/array.py`:
- Around line 302-306: Add a regression test that verifies disabling meta
tracking with set_track_meta(False) does not break channel-first conversion when
using LoadImage or LoadImaged with ensure_channel_first=True: create a test that
calls set_track_meta(False), runs LoadImage(..., ensure_channel_first=True) (and
a LoadImaged variant) on a sample array, and asserts the output is channel-first
(shape/order) and that no MetaTensor metadata is attached (or meta dict remains
absent) to ensure EnsureChannelFirst still runs on plain tensors; reference
EnsureChannelFirst, LoadImage/LoadImaged, ensure_channel_first, set_track_meta,
and MetaTensor when locating code to test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9e5bb3f7-9f63-4fb0-9f3b-14fdd624423e
📒 Files selected for processing (1)
monai/transforms/io/array.py
|
@ericspod: CI is green and the branch is up to date. Happy to answer any questions if helpful. |
ericspod
left a comment
There was a problem hiding this comment.
Hi @williams145 thanks for this fix. I think it's correct but we should have a regression test for the class as suggested. I suggested a change, please have a look at that as well.
797cdb7 to
36ded05
Compare
|
Thanks for the review. Verified the simplification works. Looking at Also added a regression test ( CI re-running now. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/transforms/test_load_image.py`:
- Around line 500-508: The test hard-codes set_track_meta(True) in the finally
block which can clobber the suite's prior state; change the test
(test_track_meta_false_ensure_channel_first) to capture the current state at the
start (e.g., prev = get_track_meta()), call set_track_meta(False) for the test,
and then in the finally block restore the original with set_track_meta(prev)
instead of set_track_meta(True); keep using LoadImage(...) and the same
assertions and ensure get_track_meta/set_track_meta symbols are used to locate
the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: aff8f650-076c-4360-8760-9219eba1c8d4
📒 Files selected for processing (2)
monai/transforms/io/array.pytests/transforms/test_load_image.py
|
Thanks for the suggestion; applied and pushed. Captured the previous state via get_track_meta() and restored it in the finally block so the test won't clobber whatever the suite had set before. |
…ck_meta is False When set_track_meta(False) is active, MetaTensor.ensure_torch_and_prune_meta returns a plain tensor. The subsequent EnsureChannelFirst() call then has no metadata source, causing a ValueError. Fix: pass meta_data explicitly when img is not a MetaTensor. Signed-off-by: UGBOMEH OGOCHUKWU WILLIAMS <williamsugbomeh@gmail.com>
…n test Apply review suggestion: pass meta_data unconditionally to EnsureChannelFirst — when img is a MetaTensor, EnsureChannelFirst overrides meta_dict with img.meta internally, so the explicit if/else is not needed. Add regression test covering set_track_meta(False) with ensure_channel_first=True on LoadImage. Signed-off-by: UGBOMEH OGOCHUKWU WILLIAMS <williamsugbomeh@gmail.com>
Capture the prior get_track_meta() value and restore it in the finally block instead of hard-coding True, so the test does not clobber state that was set by an earlier test. Per review suggestion. Signed-off-by: UGBOMEH OGOCHUKWU WILLIAMS <williamsugbomeh@gmail.com>
7d2bea2 to
045363d
Compare
Fixes #8442
Description
LoadImagewithensure_channel_first=TrueraisesValueErrorwhenset_track_meta(False)is active.Root cause chain:
set_track_meta(False)causesMetaTensor.ensure_torch_and_prune_meta()to return a plain tensor instead of aMetaTensorEnsureChannelFirst()(img)then has no metadata source (img is not a MetaTensor and nometa_dictis passed)ValueErrorbranch inEnsureChannelFirst.__call__at line 205 ofutility/array.pyFix: pass
meta_dataexplicitly toEnsureChannelFirstwhenimgis not aMetaTensor.EnsureChannelFirst.__call__already accepts ameta_dictparameter;meta_datais always a validated dict at this point inLoadImage.__call__.Types of changes
./runtests.sh --quick --unittests --disttests.make htmlcommand in thedocs/folder.