Lots of 'Invalid vr type... ' log messages in dcmjs release 0.29.11

[jbocce]

Using OHIF...

1. Open a browser window and open the browser's console window.
2. Launch the following study in OHIF's dev version https://viewer-dev.ohif.org/viewer/?StudyInstanceUIDs=2.16.840.1.114362.1.11972228.22789312658.616067305.306.2
3. Notice the many logged error messages in the console. They typically look like the following...

ValueRepresentation.js:262 Invalid vr type xs - using US
value	@	ValueRepresentation.js:262
set	@	ValueRepresentation.js:40
(anonymous)	@	DicomMetaDictionary.js:160
value	@	DicomMetaDictionary.js:117
d	@	index.js:346
(anonymous)	@	index.js:395
(anonymous)	@	index.js:439
Promise.then (async)		
(anonymous)	@	index.js:438
_retrieveSeriesMetadataAsync	@	index.js:437
await in _retrieveSeriesMetadataAsync (async)		
metadata	@	index.js:193
(anonymous)	@	Mode.tsx:67
(anonymous)	@	Mode.tsx:66
(anonymous)	@	Mode.tsx:388
(anonymous)	@	Mode.tsx:400
Ul	@	react-dom.production.min.js:262
t.unstable_runWithPriority	@	scheduler.production.min.js:18
qa	@	react-dom.production.min.js:122
Nl	@	react-dom.production.min.js:261
(anonymous)	@	react-dom.production.min.js:261
x	@	scheduler.production.min.js:16
M.port1.onmessage	@	scheduler.production.min.js:12

4. Previous versions (namely 0.29.5) did not have this.

Not sure what is causing it. Please advise.

[jbocce]

FYI @pieper

[sedghi]

Sorry for tagging you Steve, we meant to see if @dmlambo has any comment on this

[pieper]

Sorry for tagging you Steve, we meant to see if @dmlambo has any comment on this

Thanks, I'm very happy for someone else to investigate this. 👍

For background:

When I click on one of the error message I get to this code:

https://github.com/dcmjs-org/dcmjs/blob/f0dc1995f3e1d986b7e668dc4780181176e90ba6/src/ValueRepresentation.js#L253-L271

Which reference this issue thread for background:

https://github.com/dgobbi/vtk-dicom/issues/38

But I don't know why the latest changes would trigger more of those cases. Can you maybe bisect between 0.29.5 and 0.29.11 to see exactly which commit leads to the extra logging?

https://github.com/dcmjs-org/dcmjs/commit/f0dc1995f3e1d986b7e668dc4780181176e90ba6

[dmlambo]

Ah, yeah that would be my change -- it's a symptom of variable VR types, which I just found out were possible: SmallestImagePixelValue, for example.

What my change introduces is the ability to access values differently than their backing data structure, to allow PN (PersonName) type values to return dicom+json when json stringifying, and formatted strings when writing part10 dcm. As part of that change I obtain the ValueRepresentation class for the VR type and call into a function which can add the required accessors. This actually only happens with PN currently (and may indeed forever stay that way).

The good news is this doesn't affect anything; it's safe to ignore it for the time being. The bad news is the dicom standard is a bit messier than I originally thought. I'll have a look at supressing this accessor step for unknown VRs.