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.

Sign up for GitHub

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

Jump to bottom

Rewrite Generated Tag Package #147

Closed
suyashkumar opened this issue Nov 30, 2020 · 1 comment
Closed

Rewrite Generated Tag Package #147

suyashkumar opened this issue Nov 30, 2020 · 1 comment
Labels
enhancement New feature or request

Comments

@suyashkumar
Copy link
Owner

suyashkumar commented Nov 30, 2020 •
edited
Loading

For sometime, I've wanted to refactor the generation code to autogenerate the tags in the tag package. Did not block 1.0 on this, but will be a good internal upgrade. Can probably switch the generator to use the innolitics JSON dump of the DICOM standard for this, somewhat similar what I did in https://github.com/gradienthealth/dicom-protos
@suyashkumar suyashkumar added the enhancement New feature or request label Nov 30, 2020
@suyashkumar suyashkumar mentioned this issue Dec 14, 2020
Open
This was referenced Dec 28, 2020
Closed
Draft
@suyashkumar suyashkumar mentioned this issue Apr 20, 2023
Closed
@lnogueir lnogueir mentioned this issue May 27, 2024
Merged
suyashkumar pushed a commit that referenced this issue May 29, 2024
@lnogueir
Use innolitics DICOM Standard JSON dump to generate tags. (#316)
b290e17 
This PR is resolving #147.

It updates the generate_tag_definitions.py script to read the DICOM standard JSON dump from the Innolitics repo. They provide a few extra fields, so we introduce them in Tag definition, namely, Keyword and Retired. Keyword represents what Name used to represent, see comments on the field definitions for details.

Innolitics also captures all possible VRs for tags that allow multiple VR values, so I updated the writer verification code to consider that. That ended up resolving #299 as well.

This is a breaking change because some tag variables have been deleted or renamed. In particular, the command tags (group 0000) have been all deleted since Innolitics doesn't provide them. I thought that was okay because these are only related to DIMSE and should never be present in a DICOM file. I can put them back somehow if you think we should keep them, but maybe we could sort that out if we ever work on #181. But mostly, tags were added or unchanged.

I ended up writing this from scratch and not branching from #169 because I thought that was a little bit too complicated for this. But we could reconsider that design if there are requests for supporting private data dictionaries.

Misc: fixes some warnings (i.e. removed deprecated ioutil, etc.), added a helper Uint32 method.
@suyashkumar
Copy link
Owner Author

closed by #316, thanks @lnogueir!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@suyashkumar