Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update ['pixdim'] after Spacing transform in meta dict. #8269

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from

Conversation

slicepaste
Copy link
Contributor

@slicepaste slicepaste commented Dec 23, 2024

Fixes #6840

Description

According to the issue, this PR addresses on the meta dictionary data['pixdim'] of NIfTI images does not update after applying the spacing or spacingd. To align with affine, we update data['pixdim'] and keep the original metainfo in data['original_pixdim']. Additionally, this PR also update the metainfo key_{meta_key_postfix}['pixdim'] in NIfTI images, consistent with spaced_data_dict['image_meta_dict']['pixdim'] in issue #6832.

Types of changes

  • Non-breaking change (fix or new feature that would not break existing functionality).
  • Breaking change (fix or new feature that would cause existing functionality to change).
  • New tests added to cover the changes.
  • Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
  • Quick tests passed locally by running ./runtests.sh --quick --unittests --disttests.
  • In-line docstrings updated.
  • Documentation updated, tested make html command in the docs/ folder.

According to the issue, this PR addresses on the meta dictionary `data['pixdim']` of NIfTI images does not update after applying the `spacing` or `spacingd`.
To align with `affine`, we update `data['pixdim']` and keep the original metainfo in `data['original_pixdim']`.
Additionally, this PR also update the metainfo `key_{meta_key_postfix}['pixdim']` in NIfTI images, consistent with `spaced_data_dict['image_meta_dict']['pixdim']` in issue Project-MONAI#6832.

Signed-off-by: Wei_Chuan, Chiang <[email protected]>
Co-authored-by: einsyang723 <[email protected]>
Co-authored-by: IamTingTing <[email protected]>
@slicepaste slicepaste changed the title Fixes #6840 Update ['pixdim'] after Spacing transform in meta dict. Dec 23, 2024
Signed-off-by: Wei_Chuan, Chiang <[email protected]>
Signed-off-by: Wei_Chuan, Chiang <[email protected]>
Signed-off-by: Wei_Chuan, Chiang <[email protected]>
Signed-off-by: Wei_Chuan, Chiang <[email protected]>
Signed-off-by: Wei_Chuan, Chiang <[email protected]>
@@ -477,6 +477,10 @@ def pixdim(self):
return [affine_to_spacing(a) for a in self.affine]
return affine_to_spacing(self.affine)

def set_pixdim(self) -> None:
Copy link
Member

Choose a reason for hiding this comment

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

Does it make sense for this to be a method? If it's only going to be called in one place it's simple code could just be put there. If there's anticipation that this would be called by other things then that's fine.

@@ -535,6 +535,9 @@ def __call__(
dtype=dtype,
lazy=lazy_,
)
if isinstance(data_array, MetaTensor) and "pixdim" in data_array.meta:
data_array = cast(MetaTensor, data_array.clone())
Copy link
Member

Choose a reason for hiding this comment

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

Why is it necessary to clone the data array here? This is going to have a cost and I think isn't compatible with lazy resampling. Perhaps this is code that should be SpactialResample instead? @atbenmurray If you could please check if this is going to interact with laziness, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Modify pixdim to original_pixdim in meta dict
2 participants