Open dgobbi opened 7 years ago
Note also that there is a bug in vtkDICOMMetaData::ResolvePrivateTag(). It assumes that the creator tag is at the same position for each instance. ResolvePrivateTag() and ResolvePrivateTagForWriting() should take index parameters.
(Fixed in 8df723b)
If I take (1) above to its logical conclusion, then the API would be something like this:
tag = data.ResolvePrivateTag(tag, creator)
if (!data.HasCreator(tag, creator))
data.AddCreator(tag, creator))
data.Set(tag, value)
Okay, that is ugly. Too many new methods. Perhaps add methods to the Tag class:
tag = tag.Resolve(ctag) # perform the trivial arithmetic, ctag is creator tag
ctag = tag.CreatorTag() # get creator tag from a resolved private tag
Then we would have
tag = data.ResolvePrivateTag(tag, creator)
if (!data.Has(tag.CreatorTag())
data.Set(tag.CreatorTag(), creator)
data.Set(tag, value)
So that is a little better, and removes the need for ResolvePrivateTagForWriting(). The two calls to CreatorTag() are a little ugly, though, and users who don't understand the details of how private tags are implemented will be confused.
Currently the method ResolvePrivateTag(tag(g,e), creator) is used to resolve a private tag. It searches for the specified creator value within the group, and uses its position to set the first two hexadecimal digits of "e". If the creator is not found, the tag resolves to (ffff,ffff).
The idea of the (ffff,ffff) value is that it indicates an error in tag resolution (i.e. the creator element is not present). However, if the creator element is missing, it might be more robust to do one (or both) of the following: 1) find the first vacant creator slot, usually (gggg,0010), and use it to resolve the private tag. 2) assume the private tag is already resolved, if the first two digits of "e" are not 00.
The above behavior makes the getting of private attributes more robust for badly-formed data sets that are missing creator elements. The one thing we want to avoid, however, is to have the private tag resolve to an existing creator which is different from the one for which the private tag is defined. Because of this, perhaps rule (2) above should not be used.
Given this behavior (meaning (1) above), after resolving a private tag, we could find the corresponding creator tag, and then check to see whether it is present. This would be used as a diagnostic for missing creator elements. When writing a private element, we could also write the creator element at the same time as the private element, i.e. add the creator element if it is missing. This might be more intuitive than the existing "ResolvePrivateTagForWriting()" method.