dcmjs-org / dcmjs

Javascript implementation of DICOM manipulation
https://dcmjs.netlify.com/
MIT License
287 stars 108 forks source link

PN tags interpretation as union between object and string breaks DIMSE functionality #381

Closed PantelisGeorgiadis closed 4 months ago

PantelisGeorgiadis commented 4 months ago

Although clearly dcmjs was created for the DICOMWeb era, it is also used for “classic” DIMSE communication through dcmjs-dimse, in which PN tags are not interpretated as JSON objects (Alphabetic, Ideographic, Phonetic). Except if I am missing something, this interpretation is introduced as a default behavior through https://github.com/dcmjs-org/dcmjs/pull/364. Unfortunately, this breaks the DIMSE functionality, and I was wondering if there is an existing parsing option that can change this behavior. If not, how easy/hard is to introduce one?

@dmlambo, @pieper, @sedghi

dmlambo commented 4 months ago

The type (json/part10) depends on whether you call toString or you call JSON.stringify on it. The former creates a part10 string (A^B==C), the latter a DICOM json string ({Alphabetic: 'A^B', Phoenetic='c'}). The tests should give good examples of this:

            jsonDataset.PerformingPhysicianName = { Alphabetic: "Doe^John" };

            expect(String(jsonDataset.PerformingPhysicianName)).toEqual(
                "Doe^John"
            );
            expect(JSON.stringify(jsonDataset.PerformingPhysicianName)).toEqual(
                '[{"Alphabetic":"Doe^John"}]'
            );

(This applies to denaturalized datasets as well.)

In terms of adding an option to change the behaviour, it seems like an anti-pattern. You'd probably be better off adding a fix on the client side. Can you point out the exact scenario where this breaks in dcmjs-dimse? I might be able to provide guidance on a fix.

PantelisGeorgiadis commented 4 months ago

Thank you @dmlambo! Your explanation made me realize that it was easier than I thought! dcmjs-dimse is now using the latest dcmjs release and all unit tests are updated accordingly!