suever / pydicom-experimental

pydicom test
0 stars 1 forks source link

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

Open suever opened 9 years ago

suever commented 9 years ago

From bastula on January 02, 2012 14:25:01

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: http://code.google.com/p/pydicom/issues/detail?id=109

suever commented 9 years ago

From darcymason@gmail.com on January 02, 2012 13:39:30

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.

suever commented 9 years ago

From bastula on January 02, 2012 14:00:46

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).

suever commented 9 years ago

From darcymason@gmail.com on January 02, 2012 17:27:26

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.

suever commented 9 years ago

From bastula on January 08, 2012 17:42:46

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: https://code.google.com/p/dicompyler/source/detail?r=73da9833a5bd

suever commented 9 years ago

From davidj.j...@gmail.com on April 17, 2012 08:51:36

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.