[ENH] filter dicoms earlier to avoid nibabel crash with XA ill-formed mutli-planar localizer dicoms #806
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #806 +/- ##
=======================================
Coverage 82.84% 82.84%
=======================================
Files 42 42
Lines 4313 4313
=======================================
Hits 3573 3573
Misses 740 740 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Affected by this as well. Any change this could be merged?! |
|
I am also affected by this; adding my voice to the merge requests. Are we waiting on test coverage? |
heudiconv/heuristics/reproin.py
Outdated
| @@ -226,7 +226,7 @@ def _delete_chars(from_str: str, deletechars: str) -> str: | |||
|
|
|||
| def filter_dicom(dcmdata: dcm.dataset.Dataset) -> bool: | |||
| """Return True if a DICOM dataset should be filtered out, else False""" | |||
| return True if dcmdata.StudyInstanceUID in dicoms2skip else False | |||
| return True if dcmdata.get("StudyInstanceUID") in dicoms2skip else False | |||
There was a problem hiding this comment.
why this change was necessary? (the others in the file are just formatting, and should not be needed)
heudiconv/dicoms.py
Outdated
| @@ -203,6 +194,15 @@ def validate_dicom( | |||
| except AttributeError: | |||
| lgr.info("File {} is missing any StudyInstanceUID".format(fl)) | |||
| file_studyUID = None | |||
| if dcmfilter is not None and dcmfilter(mw.dcm_data): | |||
| lgr.warning("Ignoring %s because of DICOM filter", fl) | |||
| return None | |||
There was a problem hiding this comment.
I am a little confused -- it seems that you moved filtering later not earlier!
I do see that you made it so it clears/deletes signatures now later than filtering is done, so would that suffice -- just to move deletion of signatures and keeping the rest of the code as is?
There was a problem hiding this comment.
Yes, you're right, the main issue was while deleting signature on malformed dicoms. I reset the position of the call to filter dicoms and reverted the heuristics change that was no longer required.
bpinsard
force-pushed
the
enh/filter_dicom_earlier
branch
from
4ce1119 to
b31d13e
Compare
see nipy/nibabel#1392 (comment)
Apparently some of the validation of the dicom triggers nibabel.nicom that crashes on ill-formed dicoms.
Moving the dicom filter a bit before, allow to easily filter these with the heuristics
filter_dicom.I should not change anything to the results.