openmrslab / suspect

MRS processing tools
https://suspect.readthedocs.io/en/latest/
MIT License
31 stars 23 forks source link

pydicom >2.0.0 reads MRS with too many elements #160

Closed laurencejackson closed 3 months ago

laurencejackson commented 2 years ago

Hiya, I've been having an issue with suspect since upgrading pydicom to 2.2.2.

To keep my application compatible with a 3rd party library I need to use pydicom>=2.1.2. Suspect was working perfectly with the old version of my application (using pydicom==1.2.0) but I'm now running into an error when trying to read dicom data.

This issue comes up when reshaping the data_iter value in io.dicom.load_dicom

# turn the data into a numpy array
    data_iter = iter(dataset[0x5600, 0x0020])
    data = complex_array_from_iter(data_iter, shape=data_shape, chirality=-1)

The errors I get are always consistent with the test data I have available- there are always 4x too many data elements to reshape - tested with Philips and Siemens MRS dicom.

E ValueError: cannot reshape array of size 8192 into shape (2,1,1,1,1024)

E ValueError: cannot reshape array of size 524288 into shape (32,1,1,1,4096)

E ValueError: cannot reshape array of size 4096 into shape (1,1,1,1,1024)

Is this a known issue? I'm familiar with MRI but don't often work with raw MRS data, so I'm unsure how to address this. I'm happy to help with a fix if anyone is able to offer some insight as to where the missing factor of 4 is coming from!

bennyrowland commented 2 years ago

Hi @laurencejackson, sorry to hear that you are having problems. This sounds to me like a datatype issue: if I had to guess then I would say that the 4 byte elements are now being interpreted as single byte elements for some reason, so you end up with 4x as many elements. I don't understand why that should suddenly change when upgrading pydicom, especially if it is the same files that load correctly via 1.2.0 that fail on 2.2.0.

I haven't looked at pydicom in a while but I see that it has evolved a lot and the API has changed considerably for loading files, so it may be that as read_file is deprecated it has also stopped working in some unusual cases. Sadly suspect doesn't currently have any MRS DICOM test files so I can't easily test out a fix. If you have an example file that is suitable for sharing (either totally anonymised or from a phantom) that you would like to contribute then I am happy to take a look and try and get it working with new pydicom. Or you are very welcome to have a look yourself, I would recommend using a Jupyter notebook and opening the file with pydicom using read_file() and dcmread(), and looking at what you get, the datatype of the data tag etc.

laurencejackson commented 2 years ago

@bennyrowland thanks for your quick reply! That sounds like a good theory, I'm afraid I don't have much experience with handling bytestreams but I'll have a go at it and see where I get to.

I'm happy to share a dicom from a phantom that we can use for testing, so at least I can contribute a test!