Ensure StringRepresentation values are coerced to strings before being written

[jody-zeitler]

I noticed that IS values such as InstanceNumber were not appearing when examining written DICOM data. The values are read in and stored in the DicomDict as numbers but are written out as strings. Ensuring that StringRepresentation values are coerced to strings results in valid tag data.

[pieper]

Thanks for the contributions @jody-zeitler and @rvantklooster 👍

Can you have a look at this in comparison to #97 and see which actually handles the cases best? Any chance one of you could add a test? (or absent a complete test within dcmjs maybe a js snippet that generates a part10 file that shows the issue when viewed with dcmdump or pydicom.

[rvantklooster]

Both approaches fix the bug. I have a slight preference for the solution in #97. DecimalString and IntegerString are subclasses of StringRepresentation and I think it is better to handle data conversion in their own class. I also don't know what the performance hit is in approach #99, there you always convert to String (even if it is a String).

Regarding a test, I am planning to create a test to read/write a part 10 file and check if the output is the same as the input. Therefore, I will need to fix the writing of SQ tags, that is currently broken.

[jody-zeitler]

Oops didn't see #97. My thought was that everything being written as a string should be provided as a string or we run into undefined behavior. It doesn't seem like any subclasses other than IS or DS would run into this problem unless the user sets the values incorrectly e.g. a Date or object. I'm going to close this one.

I was thinking about why these types are converted to numbers in the first place. On the one hand it makes manipulation easier, but on the other hand you could run into rounding errors for DS due to a loss of precision. Something to consider for later.

[rvantklooster]

Hi @jody-zeitler . Thanks for your reply. Regarding the conversion and rounding errors, I had a look at how pydicom handles this. In pydicom they actually keep a copy of the original string next to the number. Probably they do a check if the field was modified and if that is not the case, they use the backup string.