-
Notifications
You must be signed in to change notification settings - Fork 258
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
FIX: Conditionally drop derived volumes from DWI sequences #1296
FIX: Conditionally drop derived volumes from DWI sequences #1296
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1296 +/- ##
=======================================
Coverage 92.25% 92.26%
=======================================
Files 99 99
Lines 12467 12469 +2
Branches 2563 2564 +1
=======================================
+ Hits 11502 11505 +3
Misses 642 642
+ Partials 323 322 -1 ☔ View full report in Codecov by Sentry. |
093b51a
to
3f81a96
Compare
@yarikoptic @mgxd Would either of you care to have a look? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks reasonable, though looking back at the docstrings there should be some mention/warning of this conditional removal of derived volumes
That happens a couple lines below. nibabel/nibabel/nicom/dicomwrappers.py Lines 526 to 530 in 3f81a96
|
this DICOM handling to just figure out the shape of the data is somewhat of a dark magic for me, so can't really assess on validity. But what I can say that if running with stock nibabel 5.2.0-3 and pydicom installed on debian I get 4 hits on that addressed bug and 1 other❯ /usr/bin/find dcm_* -type d | grep -v '\.git/' | while read d; do dcm=$(/bin/ls $d/*.dcm 2>/dev/null| head -n 1 || echo ); [ -z "$dcm" ] || chronic python -W ignore::UserWarning -c 'import sys; f=sys.argv[1]; print(f, end=" "); from nibabel.nicom import dicomwrappers as didw; print(didw.wrapper_from_file(f).image_shape)' $dcm; done;
dcm_qas/dcm_qa_xa30/In/11_DWI_dir80_PA/0001_1.3.12.2.1107.5.2.43.67093.2022071112123417719608653.dcm Traceback (most recent call last):
File "<string>", line 1, in <module>
File "/usr/lib/python3/dist-packages/nibabel/onetime.py", line 156, in __get__
val = self.getter(obj)
^^^^^^^^^^^^^^^^
File "/usr/lib/python3/dist-packages/nibabel/nicom/dicomwrappers.py", line 545, in image_shape
frame_indices = np.delete(frame_indices, stackid_dim_idx, axis=1)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "<__array_function__ internals>", line 200, in delete
File "/usr/lib/python3/dist-packages/numpy/lib/function_base.py", line 5136, in delete
axis = normalize_axis_index(axis, ndim)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
numpy.AxisError: axis 1 is out of bounds for array of dimension 1
dcm_qas/dcm_qa_xa30/In/12_DWI_dir80_PA/0001_1.3.12.2.1107.5.2.43.67093.2022071112123419375008659.dcm Traceback (most recent call last):
File "<string>", line 1, in <module>
File "/usr/lib/python3/dist-packages/nibabel/onetime.py", line 156, in __get__
val = self.getter(obj)
^^^^^^^^^^^^^^^^
File "/usr/lib/python3/dist-packages/nibabel/nicom/dicomwrappers.py", line 545, in image_shape
frame_indices = np.delete(frame_indices, stackid_dim_idx, axis=1)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "<__array_function__ internals>", line 200, in delete
File "/usr/lib/python3/dist-packages/numpy/lib/function_base.py", line 5136, in delete
axis = normalize_axis_index(axis, ndim)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
numpy.AxisError: axis 1 is out of bounds for array of dimension 1
dcm_qas/dcm_qa_xa30/In/19_DWI_dir80_AP/0001_1.3.12.2.1107.5.2.43.67093.2022071112140609850612301.dcm Traceback (most recent call last):
File "<string>", line 1, in <module>
File "/usr/lib/python3/dist-packages/nibabel/onetime.py", line 156, in __get__
val = self.getter(obj)
^^^^^^^^^^^^^^^^
File "/usr/lib/python3/dist-packages/nibabel/nicom/dicomwrappers.py", line 545, in image_shape
frame_indices = np.delete(frame_indices, stackid_dim_idx, axis=1)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "<__array_function__ internals>", line 200, in delete
File "/usr/lib/python3/dist-packages/numpy/lib/function_base.py", line 5136, in delete
axis = normalize_axis_index(axis, ndim)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
numpy.AxisError: axis 1 is out of bounds for array of dimension 1
dcm_qas/dcm_qa_xa30/In/20_DWI_dir80_AP/0001_1.3.12.2.1107.5.2.43.67093.2022071112140611403312307.dcm Traceback (most recent call last):
File "<string>", line 1, in <module>
File "/usr/lib/python3/dist-packages/nibabel/onetime.py", line 156, in __get__
val = self.getter(obj)
^^^^^^^^^^^^^^^^
File "/usr/lib/python3/dist-packages/nibabel/nicom/dicomwrappers.py", line 545, in image_shape
frame_indices = np.delete(frame_indices, stackid_dim_idx, axis=1)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "<__array_function__ internals>", line 200, in delete
File "/usr/lib/python3/dist-packages/numpy/lib/function_base.py", line 5136, in delete
axis = normalize_axis_index(axis, ndim)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
numpy.AxisError: axis 1 is out of bounds for array of dimension 1
dcm_qas/dcm_qa_fmap/In/Philips/fmap/IM_0027_fMAP.dcm Traceback (most recent call last):
File "<string>", line 1, in <module>
File "/usr/lib/python3/dist-packages/nibabel/onetime.py", line 156, in __get__
val = self.getter(obj)
^^^^^^^^^^^^^^^^
File "/usr/lib/python3/dist-packages/nibabel/nicom/dicomwrappers.py", line 566, in image_shape
raise WrapperError('Calculated shape does not match number of frames.')
nibabel.nicom.dicomwrappers.WrapperError: Calculated shape does not match number of frames.
with this patch (with post-installed pydicom) I get only that other `Calculated shape does not match number of frames. ❯ /usr/bin/find dcm_* -type d | grep -v '\.git/' | while read d; do dcm=$(/bin/ls $d/*.dcm 2>/dev/null| head -n 1 || echo ); [ -z "$dcm" ] || chronic /home/yoh/proj/nipy/nipy-suite/nibabel/venvs/dev3/bin/python -W ignore::UserWarning -c 'import sys; f=sys.argv[1]; print(f, end=" "); from nibabel.nicom import dicomwrappers as didw; print(didw.wrapper_from_file(f).image_shape)' $dcm; done;
dcm_qas/dcm_qa_fmap/In/Philips/fmap/IM_0027_fMAP.dcm Traceback (most recent call last):
File "<string>", line 1, in <module>
File "/home/yoh/proj/nipy/nipy-suite/nibabel/venvs/dev3/lib/python3.11/site-packages/nibabel/onetime.py", line 156, in __get__
val = self.getter(obj)
^^^^^^^^^^^^^^^^
File "/home/yoh/proj/nipy/nipy-suite/nibabel/venvs/dev3/lib/python3.11/site-packages/nibabel/nicom/dicomwrappers.py", line 569, in image_shape
raise WrapperError('Calculated shape does not match number of frames.')
nibabel.nicom.dicomwrappers.WrapperError: Calculated shape does not match number of frames. so, not sure if returned dimensionality is right, but it is definitely not hitting the error any more. |
my question would be - how feasible would be to get the release with this out? ;) or should we test more? |
Could put out a 5.2.1 with this and other bug fixes. Is the shape mismatch thing something we can address? |
on that - added a little to its own |
DATA: Add dcm_qa_xa30 as submodule for test data TEST: Add test for Siemens TRACE volume FIX: Conditionally drop isotropic frames
5.2.1 (Monday 26 February 2024) Bug-fix release in the 5.2.x series. Enhancements ------------ * Support "flat" ASCII-encoded GIFTI DataArrays (pr/1298) (PM, reviewed by CM) Bug fixes --------- * Tolerate missing ``git`` when reporting version info (pr/1286) (CM, reviewed by Yuri Victorovich) * Handle Siemens XA30 derived DWI DICOMs (pr/1296) (CM, reviewed by YOH and Mathias Goncalves) Maintenance ----------- * Add tool for generating GitHub-friendly release notes (pr/1284) (CM) * Accommodate pytest 8 changes (pr/1297) (CM) * tag '5.2.1': REL: 5.2.1 Backport gh-1296: Conditionally drop derived volumes from DWI sequences Backport gh-1298: Support "flat" ASCII-encoded GIFTI DataArrays Build(deps): Bump codecov/codecov-action from 3 to 4 Build(deps): Bump the actions-infrastructure group with 3 updates Backport gh-1297: Accommodate pytest 8 changes Backport gh-1286: Tolerate missing git MNT: Advertise Python 3.12 support DOC: Fix intersphinx mapping and reference type Backport gh-1284: Add tool for generating GitHub-friendly release notes
#795 cast a wide net, based on Phillips DICOMs that concatenated derived volumes to diffusion series. This causes problems for Siemens sequences where only derived volumes are contained in an image.
This is a pretty minimal fix that changes the rule to "if raw and derived are present, drop derived, otherwise keep whatever we have".
Fixes #1245.