search

ivoflipse / pydicom

Automatically exported from code.google.com/p/pydicom
0 stars 1 forks source link

Memory error is produced when reading a non-DICOM file #75

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
What steps will reproduce the problem?

Try to read a file that is not a dicom file.

What is the expected output? What do you see instead?

I would expect an Exception being thrown telling me the file is not dicom.
The read-function could easily check this by looking for "DICM" at position
128 in the file. Instead, a Memory error is produced.

What version of the product are you using?

0.9.4

Original issue reported on code.google.com by almar.klein@gmail.com on 16 Feb 2010 at 11:14

GoogleCodeExporter commented 9 years ago 
After examining the source code, I see that a missing header is assumed when the
"DICM" string is not found. I don't know whether this occurs often, but strictly
speaking, such a file is not valid dicom, and IMO should not be processed. 

In my application, I use CT data, which is stored in several hundred dicom 
files in a
directory. Sometimes there may be a text file or xml file with a summary of the 
data
of some sort. I could of course test for the extension, but catching a 
NotValidDicom
exception (or something like that) would be more beautiful (and would make my
application work whatever extra data I'd like to put with the dicom files).

I'd be happy to submit a patch if you agree with me.

Original comment by almar.klein@gmail.com on 16 Feb 2010 at 11:22

GoogleCodeExporter commented 9 years ago 
> almar.klein wrote: 
>... catching a NotValidDicom
> exception (or something like that) would be more beautiful (and would make my
> application work whatever extra data I'd like to put with the dicom files).
> I'd be happy to submit a patch if you agree with me.

This has been discussed in issue 70, so I'm flagging this as a duplicate. But 
yes, I'd 
be happy to accept a patch (against the new 0.9.4-1 if possible). But it should 
include using an optional argument "force=False" as discussed in issue 70, 
which (if 
set True) would retain the current behaviour, else raise an exception as you 
suggested.

Original comment by darcymason@gmail.com on 18 Feb 2010 at 3:45

GoogleCodeExporter commented 9 years ago 
Ah, I'm sorry for not properly searching the issue list before submitting...
Will look into creating a patch.

Original comment by almar.klein@gmail.com on 18 Feb 2010 at 8:33

GoogleCodeExporter commented 9 years ago 
This issue was closed by revision a7c9dce062.

Original comment by darcymason@gmail.com on 26 Feb 2010 at 3:28