dcmjs
dcmjs copied to clipboard
fix: 🐛 update invalid VR type for LUTData
Update the Value Representation (VR) for the LUTData tag. It was defined as lt instead of LT.
The method ValueRepresentation.createByTypeString(type) from ValueRepresentation.js couldn't find the right VR class from the VRInstances object, and it was creating an UnknownValue object. This was causing a crash when reading a file with that tag.
Context
Invalid vr type lt - using UN
(node:28115) UnhandledPromiseRejectionWarning: TypeError: First argument to DataView constructor must be an ArrayBuffer
at new DataView (<anonymous>)
at new BufferStream (/tmp/.mount_RadiobfjpWia/resources/app.asar/index.js:2:203740)
at ReadBufferStream._createSuperInternal (/tmp/.mount_RadiobfjpWia/resources/app.asar/index.js:2:128580)
at new ReadBufferStream (/tmp/.mount_RadiobfjpWia/resources/app.asar/index.js:2:210562)
at UnknownValue.writeBytes (/tmp/.mount_RadiobfjpWia/resources/app.asar/index.js:2:221335)
at Tag.write (/tmp/.mount_RadiobfjpWia/resources/app.asar/index.js:2:246135)
at /tmp/.mount_RadiobfjpWia/resources/app.asar/index.js:2:1105876
at Array.forEach (<anonymous>)
at Function.write (/tmp/.mount_RadiobfjpWia/resources/app.asar/index.js:2:1105724)
at SequenceOfItems.writeBytes (/tmp/.mount_RadiobfjpWia/resources/app.asar/index.js:2:235261)
(node:28115) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 3)
Tests
All unit tests pass :heavy_check_mark:
Test Suites: 10 passed, 10 total
Tests: 52 passed, 52 total
Snapshots: 0 total
Time: 5.638 s
Ran all test suites.
Thanks for the report and the proposed fix 👍
Question though: shouldn't the VR be US or OW?
https://dicom.innolitics.com/ciods/nm-image/voi-lut/00283010/00283006
Thanks for taking the time to review this @pieper
Yes, you are right!
In the case that a tag can have more than one VR, what is the best approach? Is using the first one that appears in the tag definition, US in this particular case, good enough? I can see that in dictionary.js you can only assign one VR for each tag.
That's a good question - I think the right answer is that whoever creates the element needs to provide the _vrMap like is done here.