Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect error "VR value mismatch for tag (0028,0120)[PixelPaddingValue]. Element.VR=SS, but DICOM standard defines VR to be US" #91

Closed
courteouselk opened this issue Jul 15, 2020 · 4 comments

Comments

@courteouselk
Copy link
Contributor

We receive this error when we try write down a DICOM file we have just parsed and slightly updated:

dicom.Element: VR value mismatch for tag (0028,0120)[PixelPaddingValue]. Element.VR=SS, but DICOM standard defines VR to be US

However, the standard defines both US and SS to be valid values. E.g. see: DICOM PS3.6 2020c - Data Dictionary or Pixel Padding Value Attribute.

@suyashkumar
Copy link
Owner

Thank you for reporting this!

I haven't done much with the write package since inheriting from the upstream fork (by contrast much of the read package is being rewritten, and the write package should be too at some point imo).

This is happening because right now only one VR is read in from the dicom standard into the tag map:

tagDict[Tag{0x0000, 0x0000}] = TagInfo{Tag{0x0000, 0x0000}, "UL", "CommandGroupLength", "1"}

Likely after some rewrites we should ingest multiple possible values for the padding.

For now, one option is to skip the VR verification during writing:

var SkipVRVerification Option = func(o *optSet) {

While this doesn't solve the validation, is this option sufficient for you?

@courteouselk
Copy link
Contributor Author

Hi @suyashkumar, thanks for the prompt reply. I had a look at the possibility to use that option and it seems that we could try to use it. There is just one minor problem, we use write.DataSetToFile call, and this one has a bug of not passing opts argument to subsequent write.DataSet call. I had just pushed a PR to fix this (one-liner).

@suyashkumar
Copy link
Owner

@courteouselk thank you! Merged #102, and will add a tagged version later today.

@suyashkumar
Copy link
Owner

@courteouselk the v0.4.5 tagged version includes your fix, thank you! Closing this out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants