Kiyokawa / pydicom

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

Forward slash used as a delimiter instead of a backslash #109

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
1. User submitted data that contains the value ''255/0/0'' for ROI Display Color

What is the expected output? What do you see instead?
''255/0/0'' should be written as ''255\0\0'' to be properly parsed, but is not.
Exception: Could not convert value ''255/0/0'' to VR 'IS' in tag (3006, 002a)

What version of the product are you using?
pydicom 0.9.6

This data was generated from plunc. Ideally the fix is to let the application 
authors to switch the delimiter to a backslash, but I don't know if it is being 
actively developed.

Although it is not standards compliant, would it be hard to implement detection 
of a forward slash as a possible delimiter?

Original issue reported on code.google.com by bastula on 2 Jan 2012 at 7:25

GoogleCodeExporter commented 8 years ago
Interesting... does 'plunc' mean PlanUNC? Odd to have a forward slash.

My first thoughts were to not make recognizing this standard, but to offer the 
user a flag for turning on the use of forward slash. After refreshing my memory 
on the code, though, there is a possible user-based solution even with the 
current version. Assuming you a dataset ds already created:

>>> import dicom.dataelem
>>> dicom.dataelem._backslash = "/"
>>> ds.ROIDisplayColor = "255/0/0"
>>> ds
(3006, 002a) ROI Display Color                   IS: ['255', '0', '0']

Problem is, you can only use one separator character at a time with this trick. 
If that is not enough, I could look into making something a little more user 
configurable for separator characters. Maybe instead of checking for an exact 
character, it could check for any character in a list, and the list could be 
modified by the user.

Original comment by darcymason@gmail.com on 2 Jan 2012 at 9:39

GoogleCodeExporter commented 8 years ago
Yes, I believe the user meant PlanUNC.

Thanks for the hint. I will probably end up using a try... except block and if 
it raises an exception, switch the separator to a forward slash and see if that 
works.

This is really just an edge case scenario, as I'm not sure if PlanUNC will be 
updated any time soon (the last update was in 2008 according to the website).

Original comment by bastula on 2 Jan 2012 at 10:00

GoogleCodeExporter commented 8 years ago
Well, looking at this again ... the _backslash as a variable is not universal 
in the code. When file reading, some of the processing is done in values.py and 
valuerep.py, and they use a literal backslash character, not in a variable. So 
a stable solution may not be as simple as my previous example. Let me know if 
the try...except works out.

Original comment by darcymason@gmail.com on 3 Jan 2012 at 1:27

GoogleCodeExporter commented 8 years ago
I ended up testing whether the value returns as None. If not, then I parsed the 
dataelement repval and manually separated the forward slash to decode the value 
into a list.

If you end up somehow integrating _backslash into MultiString in valuerep.py as 
well, that would fix it. I tried manually replacing the delimiter in the method 
and that worked.

My changes in dicompyler can be found here: 
http://code.google.com/p/dicompyler/source/detail?r=73da9833a5bd

Original comment by bastula on 9 Jan 2012 at 1:42

GoogleCodeExporter commented 8 years ago
I've run into this recently.  I have old dicom from 1993 that apparently uses / 
as the string delimiter It would be very benifical to have a "lenient" flag 
that checks for backslash, forwardslash or space and splits on all of them.

Original comment by davidj.j...@gmail.com on 17 Apr 2012 at 3:51