moloney / dcmstack

DICOM to Nifti conversion with meta data preservation
Other
72 stars 51 forks source link

parse_and_group error #16

Closed satra closed 10 years ago

satra commented 11 years ago

when running parse and group - with master branch - i get an error whose end point is this:

error: unpack requires a string argument of length 2
> /software/python/anaconda/envs/devpype/lib/python2.7/site-packages/dcmstack-0.7.dev-py2.7.egg/dcmstack/extract.py(348)_get_elem_value()
    347             if elem.VM == 1:
--> 348                 return struct.unpack(unpack_vr_map[elem.VR], elem.value)[0]
    349             else:

when i peek i get:

ipdb> elem.VR
'US or SS'
ipdb> unpack_vr_map[elem.VR]
'H'
ipdb> elem.value 
'\x00\x10\x00\x00\x10\x00'
ipdb> elem.VM
1
moloney commented 11 years ago

Best I can tell, this DICOM file must be invalid.

With a VR of "US or SS" we know that each value should have two bytes, and with a VM of 1 we expect a single two byte value.

Could you give some more details? For example which DICOM element this corresponds with and what software created (or last touched) the file.

satra commented 11 years ago

this is the element:

(0028, 1101) Red Palette Color Lookup Table Desc US or SS: '\x00\x10\x00\x00\x10\x00'

https://www.dabsoft.ch/dicom/3/C.7.6.3.1.5/

from reading the description, it looks like it's a tuple

and this is what i can do in the debugger:

struct.unpack(unpack_vr_map[elem.VR]*3, elem.value)
(4096, 0, 16)

and hence my suggested fix.

moloney commented 11 years ago

So it appears that the VM is wrong as it should be 3. Your proposed fix would only return the first value.

I still think the DICOM file is invalid, but I suppose a change that accounts for the VM possibly being wrong would not harm valid files.

satra commented 11 years ago

where does the VM come from? pydicom? or from the file itself? perhaps i can look into that to see if there is an issue.

i can try to use dclunie's tools to check the validity of the file

also i pushed a small change to return the appropriate number of elements.

moloney commented 11 years ago

The VM should come from the file itself. You could also possibly check the output of the 'dcmdump' command from DCMTK.

The updated PR looks reasonable at first glance. I will think about it a little more and then do the pull if I don't see issues.