Using the 'DICOMTagPath::PropertyNameToDICOMTagPath'-function, a DICOM tag with hexadecimal numbers will not be be correctly converted into a 'DICOMTagPath'.
The function does not take the letters of a hexadecimal number into account (only digits (0-9)).
Note:
A test for the DICOMTagPath ('mitkDICOMTagPathTest') did not uncover this bug, as the test cases do not contain a DICOM tag with hex-letters.
| Restricted Differential Revision |
This is a problem not only relevant for PropertyNameToDICOMTagPath(). But for all Methods in DICOMTagPath.cpp (the conversion methods and DICOMTagPath::FromStr()) that use regex to search for dicom elements (so basically all regex strings that contain "\\d").
So please do not only fix the function you mentioned but the whole cpp.
Best way (as you mentioned) would be to ensure that all tested functions are confronted with at least one hex-Element,
Confronting the test functions with hex-elements with lowercase- and uppercase-characters revealed a test error, as the std::hex manipulator converts a DICOM tag into a hex-string with lowercase-characters per default.
So a DICOM.0010.001A will be converted to DICOM.0010.001a.
Good point. Then we should add a std::uppercase manipulator to the streaming to ensure uppercase outputs. Best search for all occurences of Std::hex in DicomTagPath.cpp and add the manipulator to the streams.
I was thinking of that, but the problem is, that both a hex-tag with lowercase characters and a hex-tag with uppercase characters will be converted to a DICOMTagPath e.g..
Streaming the group or element of the tag of the DICOMTagPath will output the hex-tag with lowercase characters, regardless of the original characters, because the DICOMTagPath-tag does only contain numerical values. So DICOM.0010.001A = Group 16, Element 26 = DICOM.001.001a.
So do you mean that std::uppercase does not work?
Otherwise I don't understand the problem.
If we are not so strict when parsing into a DICOMTagPath, I don't see a problem. But we should only convert DICOMTagPath into upsercase strings. Or did I get something wrong.
We could create a DICOM tag with with either a lowercase character or a uppercase character. This DICOM tag is then converted into a group and element pair, which contains the decimal values. If we now want to convert these back to hex with the std::hex modificator, we don't know, if the original DICOM tag had lowercase or uppercase characters. So we cannot correctly assert a DICOM tag string (DICOM.XXXX.YYYY) with the std::hex output, as this will be lowercase per default (but the original DICOM tag may have been either lowercase or uppercase).
[Actually I'm not sure if this is really a problem, as we may accept that each tag we create inside the program is lowercase (or uppercase, as you proposed), regardless of the original input DICOM tag.]
Edit: Actually I'm sure this is a problem:
Using e.g. 'DICOMTagPathToPropertyRegEx' we want to create a regEx from a DICOM Tag. If the original DICOM tag contained lowercase characters, the regular expression should only match the same DICOM tag with lowercase characters and vice versa.
Looking at http://dicom.nema.org/Dicom/2013/output/pdf/part06.pdf on page 21, the standard has uppercase-characters.
So I agree with your suggestions to convert DICOMTagPath into uppercase strings, but parse lowercase and uppercase strings.
This task was resolved by diff D17.
Branch 'T22064-Convert-DICOM-hex-tags exists', but the final diff was directly pushed to master via arc diff and arc land.
Needs to be considered for MITK Release 2016-11