Skip to content

Add DICOM support for volume additions#398

Merged
arhowe00 merged 6 commits into
mainfrom
396-Enable-adding-volume-from-DICOM
Nov 20, 2025
Merged

Add DICOM support for volume additions#398
arhowe00 merged 6 commits into
mainfrom
396-Enable-adding-volume-from-DICOM

Conversation

@arhowe00
Copy link
Copy Markdown
Contributor

@arhowe00 arhowe00 commented Nov 10, 2025 •
edited
Loading

Closes #396

Dicom series and dicom files are converted to nifti format and stored in the database.

Data

The added test data comes from the data used for pydicom's unit testing:

CT_small.dcm:

dicom_series files (6293 and 6924):

@ebrahimebrahim
Copy link
Copy Markdown
Collaborator

ebrahimebrahim commented Nov 10, 2025

nice!!

before i review just quickly:

  • remember to mention issue # in the commit
  • what is the source of the test data? just making sure we have permission to use it

@ebrahimebrahim ebrahimebrahim linked an issue Nov 11, 2025 that may be closed by this pull request
Enable adding volume from DICOM #396
Closed
@arhowe00 arhowe00 force-pushed the 396-Enable-adding-volume-from-DICOM branch from d6321e1 to 3a5702e Compare November 11, 2025 14:34
@arhowe00
Copy link
Copy Markdown
Contributor Author

arhowe00 commented Nov 11, 2025

nice!!

before i review just quickly:

  • remember to mention issue # in the commit
  • what is the source of the test data? just making sure we have permission to use it

I added the data source to the PR description. Also, I reworded the comits.

@ebrahimebrahim
Copy link
Copy Markdown
Collaborator

ebrahimebrahim commented Nov 11, 2025

Thanks for sharing data sources. Based on the readme at https://github.com/pydicom/pydicom/tree/a68344bff1caaa4a5ea17946b233ae07f02056e4/src/pydicom/data/test_files it seems we are free to use the data files here.

ebrahimebrahim
ebrahimebrahim requested changes Nov 11, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@ebrahimebrahim ebrahimebrahim left a comment

Choose a reason for hiding this comment

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

Great work! We just need to work the affine into this now and not later, because the wrong affine can be dangerous.

Note to self: in the next review test this in SlicerOpenLIFU

Comment thread src/openlifu/db/database.py Outdated
Comment thread src/openlifu/db/database.py Outdated
ebrahimebrahim added a commit to OpenwaterHealth/SlicerOpenLIFU that referenced this pull request Nov 17, 2025
@ebrahimebrahim
hack to test P#398
b880246 
See OpenwaterHealth/openlifu-python#398
ebrahimebrahim
ebrahimebrahim requested changes Nov 17, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@ebrahimebrahim ebrahimebrahim left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for the updates

I tried testing this in SlicerOpenLIFU itself, and it looks like something is off with the affine:

image

Here's how I got that screenshot:

  • Used Slicer to convert the nifti volume in our dvc data to DICOM. Here is the resulting dicom (shared with @arhowe00)
  • Made a tweak to SlicerOpenLIFU to enable specifying a folder rather than a file when adding volume. (Later we will have to come up with a way to allow both)
  • Used the add volume dialog in SlicerOpenLIFU to add a volume. This internally calls Database.write_volume so I could have just done that instead of using SlicerOpenLIFU.
  • Load the volume (which has now been converted to NIFTI) back into Slicer

The right affine metadata is definitely there in the dicom file, because when I used Slicer's dicom importer it came out matching the original nifti.

So something must be wrong with the affine handling in the dicom-to-nifti conversion in this PR

@ebrahimebrahim ebrahimebrahim mentioned this pull request Nov 17, 2025
Closed
@arhowe00
Copy link
Copy Markdown
Contributor Author

arhowe00 commented Nov 19, 2025

I tried testing this in SlicerOpenLIFU itself, and it looks like something is off with the affine:

Nice catch! I did not rotate the affine from LPS to RAS in patient-space, which should be done when loading the volume as nifti. I fixed this and tested it, in CLI, I also improved the tests to be more flushed out.

@arhowe00 arhowe00 force-pushed the 396-Enable-adding-volume-from-DICOM branch from 4013d1f to 23909aa Compare November 19, 2025 19:07
ebrahimebrahim
ebrahimebrahim approved these changes Nov 20, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@ebrahimebrahim ebrahimebrahim left a comment

Choose a reason for hiding this comment

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

The fix worked! I added comments and you can feel free to deal with or ignore them, then please go ahead and hit "rebase and merge" when you are happy. 😄

Comment thread src/openlifu/util/volume_conversion.py Outdated
Comment thread src/openlifu/util/volume_conversion.py Outdated
@arhowe00
Add DICOM to NIfTI conversion support (#396)
a6bc366 
- Add pydicom dependency to handle DICOM input files
- Implement is_dicom_file_or_directory() to detect DICOM files/directories
- Implement convert_dicom_to_nifti() for format conversion
- Update write_volume() to automatically convert DICOM to NIfTI
- Add comprehensive tests and testing data
  - A DICOM series containing 2 slices (`tests/resources/dicom_series/`)
  - A DICOM file containing a CT (`tests/resources/CT_small.dcm`)
@arhowe00
Simplify slice processing (#396)
2b2fcd3
@arhowe00
Extract DICOM conversion to util module and add proper affine matrix (#…
b8d2e03 
…396)

- Move is_dicom_file_or_directory() and convert_dicom_to_nifti() from
  database.py to new util/volume_conversion.py
- Implement extract_affine_from_dicom() using ImageOrientationPatient,
  ImagePositionPatient, and PixelSpacing from DICOM headers
- Create tests/test_volume_conversion.py with unit tests for conversion
  functions
- Remove conversion function tests from test_database.py, keep
  integration tests for write_volume
- Update imports in database.py and test_database.py
@arhowe00
Fix pylint warnings (#297)
199e1ad 
```
nox > pylint openlifu
************* Module openlifu.db.database
Warning: .nox/pylint/lib/python3.12/site-packages/openlifu/db/database.py:396:24: R1732: Consider using 'with' for resource-allocating operations (consider-using-with)
************* Module openlifu.nav.photoscan
Warning: .nox/pylint/lib/python3.12/site-packages/openlifu/nav/photoscan.py:195:13: I1101: Module 'OpenEXR' has no 'File' member, but source is unavailable. Consider adding this module to extension-pkg-allow-list if you want to perform analysis based on run-time introspection of living objects. (c-extension-no-member)
```
@arhowe00
Add LPS -> RAS conversion for affine in dicom -> nifti pipeline (#396)
b05a444
@arhowe00
Improve code clarity in volume_conversion module (#396)
e9157a5 
Renamed 'dicom_dataset' and 'ds' to 'header' throughout for better
readability. Added complete type annotation for dicom_slices parameter
and clarified in docstring that header is a pydicom.Dataset containing
DICOM metadata tags.
@arhowe00 arhowe00 force-pushed the 396-Enable-adding-volume-from-DICOM branch from 928e68d to e9157a5 Compare November 20, 2025 16:14
@arhowe00 arhowe00 merged commit 4c58110 into main Nov 20, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ebrahimebrahim ebrahimebrahim ebrahimebrahim approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Enable adding volume from DICOM

2 participants

@arhowe00 @ebrahimebrahim