ENH: Multi frame filter to handle stack with multiple orientations

[octomike]

Some vendors produce enhanced DICOMs with (logical) frame groups of different orientations sharing the same Stack ID - this breaks in image_shape.

This filter groups frames by orientation and returns only the frames of the first logical stack.

Relevant for #1392

I basically hit #1392 with some XA70A localizer data from a recent Siemens scanner. Unfortunately filtering in downstream applications (heudiconv) is not possible for us, because we rely on the localizer name to set a session ID (reproin logic).

I'm aware this PR does not solve the issue. Still, it's super helpful for us and maybe to others as well. The code is emitting a warning that frames have been discarded, similar to the two existing filters. Also, I have trouble interpreting the definition of the Stack ID and I'm still not 100% certain this is actually allowed in DICOM or if our data is non-conformant.

Be aware, that the method was drafted by claude/AI with me reviewing and testing it.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.44%. Comparing base (09bdbf4) to head (db8e0fc).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1485   +/-   ##
=======================================
  Coverage   95.44%   95.44%           
=======================================
  Files         209      209           
  Lines       29981    30042   +61     
  Branches     4483     4493   +10     
=======================================
+ Hits        28614    28675   +61     
  Misses        931      931           
  Partials      436      436           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[effigies]

It would be best if there was a test that currently failed and then was fixed by this (or another) change. See https://www.codecademy.com/article/tdd-red-green-refactor. We can add a test file to https://github.com/nipy/nibabel/tree/master/nibabel-data (it would go in https://github.com/effigies/nitest-dicom and should be stripped of any PII).

@bpinsard Could I ask you to review the code? I have almost no practical experience with DICOMs, so if I'm reviewing it will be based on testing alone.
@moloney Your input is always welcome, as well.

[octomike]

Can do. I would create a new PR with a test and expect a failure. Then, rebase this PR and change the expectation to success. Does that make sense?

[effigies]

Whatever process works for you. I'm not super worried as long as it's verifiable.

[bpinsard]

I think the fix makes complete sense. Have you tested downstream software: notably heudiconv and the potential mismatch between the output nifti of dcm2niix (with the -i o flag or not) and the resulting MultiframeDicomWrapper shape.

[moloney]

Would be good to mimic the FilterMultiStack class and give the filter an __init__ method and allow the user to choose which orientation to keep (I guess by index where 0 is first unique iop, 1 is the next unique iop...). That way it is possible to get all the data out of the file by iterating this index.

[octomike]

Would be good to mimic the FilterMultiStack class and give the filter an __init__ method and allow the user to choose which orientation to keep (I guess by index where 0 is first unique iop, 1 is the next unique iop...). That way it is possible to get all the data out of the file by iterating this index.

@moloney Good idea. I have added this and also removed the warning when a user explicitly requests a frame group.

[octomike]

I think the fix makes complete sense. Have you tested downstream software: notably heudiconv and the potential mismatch between the output nifti of dcm2niix (with the -i o flag or not) and the resulting MultiframeDicomWrapper shape.

I already use this patch to convert a dataset using heudiconv. But to be honest, the impact I test this way is very limited, because we're talking only about a localizer, which is usually discarded anyway. Before, heudiconv failed in validate_dicom and all locations, where image_shape was read from the image. It's not converting to nifti, but it needs the metadata in the dicom.

Right now, there will always be a mismatch to dcm2niix - because we are only returning a subset of the frames (first group by default). With dcm2niix' development branch I get:

option	output
dcm2niix -i y	Error: DICOM incompatible with NIfTI slice orientation - bug in dcm2niix?, it should be ignored
dcm2niix -i n	Error: DICOM incompatible with NIfTI slice orientation
dcm2niix -i o	Warning: Keeping series even though slice orientation varies (all groups stacked)
this PR	Keep only the first (or some other) group in the series

[effigies]

I think I understand this, and it makes reasonable sense to me.

@octomike Can you merge your commit from #1486 into this? I think the shape should be (512, 512, 7), not (512, 512, 3).

It would also be a good use of tests to demonstrate how the feature should actually be used. Here's what I was able to figure out:

    @dicom_test
    @needs_nibabel_data('dcm_qa_xa60')
    def test_data_multi_orient_extract_all(self):
        # Test that a file with 3 orientations sharing the same StackID
        with ImageOpener(DATA_FILE_XA60_MULTI_ORIENT) as fobj:
            dcm_data = pydicom.dcmread(fobj)
        for keep_group, nslices in enumerate([7, 3, 3]):
            dw = didw.wrapper_from_data(
                dcm_data, frame_filters=[didw.FilterMultiOrient(keep_group=keep_group)]
            )
            assert dw.image_shape == (512, 512, nslices)

If other ergonomics are intended, please use those.

[octomike]

@effigies done and I included your test - makes total sense to test that feature as well

[octomike]

Rebased on master (and included the typo fix for DEFUALT_FRAME_FILTERS)