Add the six registered UIDs pydicom's dictionary is missing. - #79
Merged
Merged
Conversation
uid's dictionary is generated from pydicom's _uid_dict.py, which is the right source for a port checked against pydicom -- but pydicom is not the authority on which UIDs exist, and it is behind PS3.6 by six Storage SOP Classes. A consumer resolving a SOP Class UID it received over the wire got the raw digits back from UID.Name and nothing from uid.Lookup. Rather than add the two named in #77 and stop, diff the whole registry: parse Tables A-1 and A-2 out of part06.xml and compare against the generated dictionary. 496 registered UIDs against 490 generated ones, and those six were the entire difference -- no extra UIDs on the Go side, no name, keyword or retired-flag disagreement anywhere else. So the four beyond #77 are: 1.2.840.10008.5.1.4.1.1.2.3 CT Image Storage - For Processing 1.2.840.10008.5.1.4.1.1.2.4 Enhanced CT Image Storage - For Processing 1.2.840.10008.5.1.4.1.1.2.5 Legacy Converted Enhanced CT ... - For Processing 1.2.840.10008.5.1.4.1.1.601.5 Ultrasound Waveform Storage All six are absent from pydicom at the pinned submodule and at upstream main 0e98c4a (2026-08-01), so bumping the submodule would not have helped. They are supplied from a STANDARD_ADDITIONS table in generate_uid_dict.py, not by editing the generated file, so a regeneration keeps them. The table is not allowed to become a quiet second source of truth: merging refuses to run once pydicom defines any of these UIDs, and it rejects an addition whose keyword is not a valid Go identifier, is reserved to MANUAL_CONSTS, is already taken by a pydicom entry, or collides with another addition -- each of which would otherwise put a UID in the dictionary and silently emit no constant for it. All five refusals are checked before anything is written, so a bad entry cannot leave a half-regenerated tree. audit_uid_dict.py is the diff, kept as a tool so the claim above is reproducible rather than something a reader has to take on trust. It exits non-zero on any discrepancy. It is not wired into CI on purpose: it depends on the network and on a document that changes without reference to this repository, so a new DICOM edition would turn an unrelated pull request red. TestUIDsAheadOfPydicom is the Go-side guard. These six are the only dictionary entries that do not come from the parse, which makes them the only ones a pydicom bump could drop with every other test still passing -- the count would just fall by six. It checks all three generated maps, since they are emitted by separate loops and an entry can survive in one and vanish from another; verified by deleting one Dictionary entry and confirming the test goes red while Lookup still passed. The generator now also runs gofmt on uid/generated.go. That file does not match CI's '*_generated.go' exclusion, so it has to be formatted, and the constants are emitted one per line for gofmt to align -- without this a regeneration shows up as a ~1000-line whitespace diff on top of the real change. Closes #77 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #77.
What was wrong
uid's dictionary is generated from pydicom's_uid_dict.py. That is the right source for a port checked against pydicom, but pydicom is not the authority on which UIDs exist, and it is behind PS3.6 by six Storage SOP Classes. A consumer resolving a SOP Class UID it received over the wire got the raw digits back fromUID.Nameand nothing at all fromuid.Lookup.Six, not two
#77 named the two Waveform Presentation State classes. Rather than add those and stop, I diffed the whole registry — parsed Tables A-1 and A-2 out of
part06.xmland compared againstuid/dictionary_generated.go:So the generator was already sound; only these late additions were missing. The four beyond #77:
1.2.840.10008.5.1.4.1.1.2.31.2.840.10008.5.1.4.1.1.2.41.2.840.10008.5.1.4.1.1.2.51.2.840.10008.5.1.4.1.1.601.5All six are absent from pydicom both at the pinned submodule and at upstream
main0e98c4a (2026-08-01), so bumping the submodule would not have helped.Keeping the mirror a mirror
They are supplied from a
STANDARD_ADDITIONStable ingenerate_uid_dict.pyrather than by editing the generated file, so a regeneration keeps them. The table is not allowed to become a quiet second source of truth — merging refuses to run when:MANUAL_CONSTS2–5 each describe a case where the constant loop would skip the keyword, putting a UID in the dictionary while silently emitting no constant for it. All five checks run before anything is written, so a bad entry cannot leave a half-regenerated tree. I verified all five fire and that the happy path merges exactly six.
Verification
audit_uid_dict.pyis the diff, kept as a tool so the numbers above are reproducible rather than something you have to take on trust. Exits non-zero on any discrepancy; currently reports496 / 496, no discrepancies. Confirmed non-vacuous by deleting an entry and watching it report that one UID and exit 1.TestUIDsAheadOfPydicomis the Go-side guard. These six are the only dictionary entries that do not come from the parse, which makes them the only ones a pydicom bump could drop with every other test still green — the count would just fall by six. It checks all three generated maps, because they come from separate loops and an entry can survive in one while vanishing from another. Verified by deleting oneDictionaryentry: the test went red onName/Type/KeywordwhileLookupstill passed, which is exactly that failure mode.Also
The generator now runs
gofmtonuid/generated.go. That filename does not match CI's*_generated.goexclusion, so it has to be formatted, and the constants are emitted one per line for gofmt to align. Without this a regeneration shows up as a ~1000-line whitespace diff on top of the real change — I hit exactly that while making this one.dictionary_generated.gois left unformatted, as it always has been, since it is excluded.The diff to the generated files is 18 added lines and nothing removed: 6
Dictionaryentries, 6KeywordToUIDentries, 6 constants.Checks run locally
go build,go vet,staticcheck -checks=all(clean), gofmt,go test -count=1 -p 4 ./..., and the full suite underGOARCH=386— all pass.🤖 Generated with Claude Code