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

feat(vr): Save original non-standard VR #414

Merged
merged 2 commits into from
Dec 19, 2024

Conversation

alekschernof
Copy link
Contributor

Hello, I encountered several issues with saving the dataset to a file.
After investigation I found that the source file has different vr that in dictionary And the main problem is that after the naturalizeDataset process we lose origin vr for these tags and in the denaturalizeDataset process we try to process tag with incorrect vr
and therefore we get the following errors when trying to write.

image

In dicom image (LUTData tag has incorrect vr. In standart it US)

Screenshot from 2024-12-18 15-09-51

We received that image and we get the following errors when trying to write

Screenshot from 2024-12-18 12-23-12

Copy link

netlify bot commented Dec 18, 2024

Deploy Preview for dcmjs2 ready!

Name Link
🔨 Latest commit 51ac8cd
🔍 Latest deploy log https://app.netlify.com/sites/dcmjs2/deploys/6764253b9c4b4600086c41fb
😎 Deploy Preview https://deploy-preview-414--dcmjs2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@pieper
Copy link
Collaborator

pieper commented Dec 18, 2024

This sounds reasonable, but a couple things.

First, we need to have commits follow the commit message format. Here it would be something like feat(vr): Save original non-standard VR and also including a summary of the change in the rest of the commit message so that it's explained in git log without needing to refer back to the PR.

Also, please add a test for this. It can be simple, just make a dataset from scratch that has the incorrect VR and then run it through the round trip (go from namified javascript object to part 10, then back from part10 to namified object and back to part 10 and then back to namified and confirm that desired VR has been preserved).

@alekschernof alekschernof changed the title Save origin vr if it different that in dictionary feat(vr): Save original non-standard VR Dec 19, 2024
…tionary and use saved VR in processing denaturalizeDataset function
@alekschernof
Copy link
Contributor Author

This sounds reasonable, but a couple things.

First, we need to have commits follow the commit message format. Here it would be something like feat(vr): Save original non-standard VR and also including a summary of the change in the rest of the commit message so that it's explained in git log without needing to refer back to the PR.

Also, please add a test for this. It can be simple, just make a dataset from scratch that has the incorrect VR and then run it through the round trip (go from namified javascript object to part 10, then back from part10 to namified object and back to part 10 and then back to namified and confirm that desired VR has been preserved).

Hello, I renamed pr and commits by requirements and added simple test for naturalizeDataset and denaturalizeDataset with checks

Copy link
Collaborator

@pieper pieper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks LGTM 👍

@pieper pieper merged commit 14a449a into dcmjs-org:master Dec 19, 2024
14 checks passed
Copy link

🎉 This PR is included in version 0.37.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

2 participants