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

[DAR-4924][External] Resolving issues with import & export of NifTI annotations #979

Merged
merged 4 commits into from
Dec 17, 2024

Conversation

JBWilkie
Copy link
Collaborator

@JBWilkie JBWilkie commented Dec 11, 2024

Problem

This PR tackles 3 issues related to importing & exporting annotations in the NifTI format. Issue 1 is major, issues 2 & 3 are minor

  • 1: NifTI annotations are being incorrectly oriented upon import and export in some situations
  • 2: It is not possible to import NifTI annotations to items in dataset folders
  • 3: When importing NifTI annotations as polygons, the last frame is always omitted

Solution

To tackle these problems:

  • 1: We were re-orienting NifTI annotations incorrectly, and in places we didn't need to be. This PR changes the logic to be the following:

When importing NifTI annotations: If importing to a post MED_2D_VIEWER file, re-orient to LPI according to nibabel. This is the exact same logic we apply to medical file processing in the backend. Otherwise, if importing to a pre MED_2D_VIEWER file:

  • If it's a NifTI dataset item: Re-orient to LPI, since all pre MED_2D_VIEWER NifTI files were re-oriented this way
  • If it's a DICOM dataset item, look up current affine of the target dataset item, and re-orient the annotations based on that

When exporting NifTI annotations: Always re-orient the exported NifTI file based on the original affine of the dataset item it came from

  • 2: Allow import of NifTI annotations to items in dataset folders
  • 3 Resolves the issue with removing the last frame of NifTI polygon annotations

IMPORTANT NOTE: Due to what I believe were some issues in the pre MED_2D_VIEWER backend processing pipeline, we may have incorrectly stored affines and original affines for some pre MED_2D_VIEWER files. Since this is all we have to go off when re-orienting NifTI annotations, there may be issues in these cases, and we should keep an eye out for them

Changelog

Resolved issues with automatic scaling & orientation of NifTI annotations

Copy link

linear bot commented Dec 11, 2024

The volume containing the affine and original affine
"""
if volume.original_affine is not None:
img_ax_codes = nib.orientations.aff2axcodes(volume.affine)
Copy link
Contributor

@dorfmanrobert dorfmanrobert Dec 11, 2024

Choose a reason for hiding this comment

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

I don't think this is correct for pre MED_2D due to the volume.affine being (or expected to be at least) RAS

So this flow will try to go from RAS -> original orientation.

But really its meant to go from LPI -> original orientation. I think this is why dataobj=np.flip(volume.pixel_array, (0, 1, 2)).astype(np.int16) used to be invoked: to go from LPI to RAS, so that then this logic would make sense for pre MED_2D files

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated

ax_codes = nib.orientations.aff2axcodes(affine)
ornt = nib.orientations.axcodes2ornt(ax_codes)
if not is_dicom:
img = nib.Nifti1Image(np.flip(img.get_fdata(), (0, 1, 2)), affine)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it might be more robust to do as_closest_canonical on img to get it to RAS, since this will only work if the Nifti annotation file is in LPI right

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated

@dorfmanrobert
Copy link
Contributor

Having some comments on why we need these transforms for pre MED_2D would be helpful for future ref I think

Copy link
Contributor

@dorfmanrobert dorfmanrobert left a comment

Choose a reason for hiding this comment

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

nice work!

@JBWilkie JBWilkie merged commit d0913e7 into master Dec 17, 2024
20 checks passed
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.

2 participants