Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 39 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Added
- Six Storage SOP Class UIDs that PS3.6 Table A-1 registers but pydicom's
`_uid_dict.py` does not carry, so `uid.Lookup` and `UID.Name` resolve them
`_uid_dict.py` does not carry, so `uid.LookupKeyword` and `UID.Name` resolve them
instead of returning the raw UID string: `CTImageStorageForProcessing`
(`1.2.840.10008.5.1.4.1.1.2.3`), `EnhancedCTImageStorageForProcessing` (`.2.4`),
`LegacyConvertedEnhancedCTImageStorageForProcessing` (`.2.5`),
Expand Down Expand Up @@ -121,8 +121,38 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
nothing about the bytes to object to. The README has a section on it with
`ExampleDataset_PrivateBlock` and `ExampleDataset_NewPrivateBlock` behind it, so
the snippets are compile-checked
- `uid.LookupKeyword(keyword string) (UID, bool)` resolves a PS3.6 keyword to its
UID — the direction the old `uid.Lookup` went, under a name that says which
direction that is. It replaces the exported `uid.KeywordToUID` map for the one
thing a caller could do with it. The generated constants remain the better answer
for a keyword known at compile time; this is for one that arrives at run time,
from a configuration file or a command line

### Changed
- **Breaking:** `uid.Lookup` takes a UID and returns what the registry records
about it — `Lookup(u UID) (Info, bool)` — where it used to take a keyword and
return a UID. That direction is now `uid.LookupKeyword`, so both exist and each
says which way it goes. The new `Lookup` is what replaces the exported maps: it
answers everything `uid.Known` was reachable for, including the four derived
transfer-syntax flags, and it hands back a copy, so the table cannot be reached
through it
- **Breaking:** the three exported maps in `uid` are gone from the API —
`Dictionary` and `KeywordToUID` become `dictionary` and `keywordToUID`, `Known` is
removed outright (see Removed), and `uid.DictEntry` becomes `dictEntry` with the
table it types. Same defect as the data dictionary above: read-only by
convention, mutable by type, no synchronisation of any kind, so any caller could
have rewritten what a transfer syntax means for every other caller in the
process — `uid.Known[uid.ImplicitVRLittleEndian]` was one assignment away from
making every implicit-VR file in the process decode as explicit. Read access is
`uid.Lookup` and `uid.LookupKeyword`. `generate_uid_dict.py` emits the unexported
names, so a regeneration cannot quietly export them again. Within the
godicom-dev organisation the only references are two lines in one gonetdicom
test, `ae/storage_sop_classes_test.go:35` and `:121`, which a module bump there
will need to move to `uid.Lookup` / `uid.LookupKeyword`
- **Breaking:** the root aliases `godicom.UIDDictionary` and `godicom.KnownUIDs` are
removed rather than unexported, there being nothing left for them to alias.
`godicom.LookupUID` is unchanged and still takes a keyword; `godicom.UIDInfo`
still names `uid.Info`, which is what `uid.Lookup` returns
- the VR-mismatch diagnostic is judged against the dictionary the read was given
rather than always against PS3.6. A caller who overrode an entry said what they
expect the file to contain, and measuring against a different expectation than the
Expand Down Expand Up @@ -202,6 +232,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
caches across the overwrite

### Removed
- the `uid.Known` map and the package `init` that filled it. It was a second copy
of the UID dictionary — one `Info` per registered UID, all 496 of them built at
program start whether anything asked or not — and every field in it is either
read straight from the dictionary entry or derived from the UID in three
comparisons. `uid.Lookup` derives them on demand instead, so there is one table
where there were two, nothing to keep in step, and no init work for a program
that never looks a UID up. `TestLookupCoversEveryEntry` checks every entry
resolves, which is the invariant `len(Known) == len(Dictionary)` used to state
- `dictionaryIsRetired`, which consulted only the exact table and so answered "not
retired" for all 72 retired repeating-group entries. It had no caller outside its
own test, so nothing ever noticed. `Standard().Lookup` returns the entry and
Expand Down
2 changes: 1 addition & 1 deletion decode.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ func DecodeDataset(data []byte, ts UID) (*Dataset, error) {

// DecodeDatasetContext is like DecodeDataset but uses ctx for logging.
func DecodeDatasetContext(ctx context.Context, data []byte, ts UID) (*Dataset, error) {
info, known := uid.Known[ts]
info, known := uid.Lookup(ts)
if !known || !info.IsTransferSyntax {
return nil, fmt.Errorf(
"godicom: Transfer Syntax UID %q is not a known transfer syntax; use DecodeDatasetEncoding",
Expand Down
12 changes: 6 additions & 6 deletions generate_uid_dict.py
Original file line number Diff line number Diff line change
Expand Up @@ -224,17 +224,17 @@ def main():
dict_lines.append("")
dict_lines.append("// Code generated by generate_uid_dict.py. DO NOT EDIT.")
dict_lines.append("")
dict_lines.append("// DictEntry holds metadata for a registered DICOM UID.")
dict_lines.append("type DictEntry struct {")
dict_lines.append("// dictEntry holds metadata for a registered DICOM UID.")
dict_lines.append("type dictEntry struct {")
dict_lines.append("\tName string")
dict_lines.append("\tType string")
dict_lines.append("\tExtraInfo string")
dict_lines.append("\tRetired bool")
dict_lines.append("\tKeyword string")
dict_lines.append("}")
dict_lines.append("")
dict_lines.append("// Dictionary maps UID values to their metadata.")
dict_lines.append("var Dictionary = map[string]DictEntry{")
dict_lines.append("// dictionary maps UID values to their metadata.")
dict_lines.append("var dictionary = map[string]dictEntry{")
for uid in sorted(uid_dict.keys()):
name, uid_type, info, retired, keyword = uid_dict[uid]
retired_bool = "true" if retired == "Retired" else "false"
Expand All @@ -245,8 +245,8 @@ def main():
)
dict_lines.append("}")
dict_lines.append("")
dict_lines.append("// KeywordToUID maps UID keywords to values.")
dict_lines.append("var KeywordToUID = map[string]UID{")
dict_lines.append("// keywordToUID maps UID keywords to values.")
dict_lines.append("var keywordToUID = map[string]UID{")
for uid in sorted(uid_dict.keys()):
keyword = uid_dict[uid][4]
if keyword == "":
Expand Down
10 changes: 2 additions & 8 deletions uid.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,12 @@ const (
GodicomImplementationUID UID = uid.GodicomImplementationUID
)

// UIDInfo holds metadata about a UID.
// UIDInfo holds metadata about a UID, as returned by [uid.Lookup].
type UIDInfo = uid.Info

// UIDDictionary maps UID values to their metadata.
var UIDDictionary = uid.Dictionary

// KnownUIDs maps UID strings to their info.
var KnownUIDs = uid.Known

// LookupUID returns the UID for a dictionary keyword.
func LookupUID(keyword string) (UID, bool) {
return uid.Lookup(keyword)
return uid.LookupKeyword(keyword)
}

// ValidateUID checks if the UID string conforms to DICOM rules.
Expand Down
12 changes: 6 additions & 6 deletions uid/dictionary_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

86 changes: 55 additions & 31 deletions uid/uid.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,12 @@ const (
VerificationSOPClass = Verification
)

// Info holds metadata about a UID (legacy shape for KnownUIDs consumers).
// Info is what Lookup returns: what the PS3.6 UID registry records about a UID,
// plus the encoding facts that follow from it.
//
// The four flags are only meaningful for a Transfer Syntax and are false for
// everything else, so IsTransferSyntax is the one to check first -- a SOP Class
// is not big-endian, it simply has no endianness at all.
type Info struct {
UID UID
Name string
Expand All @@ -46,39 +51,58 @@ type Info struct {
IsLittleEndian bool
}

// Known maps UID strings to their metadata. Populated from Dictionary.
var Known map[UID]Info

func init() {
Known = make(map[UID]Info, len(Dictionary))
for value, entry := range Dictionary {
u := UID(value)
info := Info{
UID: u,
Name: entry.Name,
Type: entry.Type,
ExtraInfo: entry.ExtraInfo,
Retired: entry.Retired,
Keyword: entry.Keyword,
}
if entry.Type == "Transfer Syntax" {
info.IsTransferSyntax = true
info.IsCompressed = u.isCompressedTransferSyntax()
info.IsImplicitVR = u == ImplicitVRLittleEndian
info.IsLittleEndian = u != ExplicitVRBigEndian
}
Known[u] = info
}
}

func (u UID) entry() (DictEntry, bool) {
e, ok := Dictionary[string(u)]
func (u UID) entry() (dictEntry, bool) {
e, ok := dictionary[string(u)]
return e, ok
}

// Lookup returns the UID for a dictionary keyword.
func Lookup(keyword string) (UID, bool) {
u, ok := KeywordToUID[keyword]
// Lookup returns what the UID registry records about u, and whether it holds an
// entry for it at all. A UID it does not know -- a private one, or a typo -- is
// reported as absent rather than answered with a zero Info, which would read like
// a registered UID that happens to be no transfer syntax:
//
// info, ok := uid.Lookup(ts)
// if !ok || !info.IsTransferSyntax {
// // ts is not something to encode with
// }
//
// The Info is a copy, so the registry cannot be reached through it. That is the
// point: this table used to be an exported map, which let any caller anywhere in
// the process redefine what a transfer syntax means for every other caller.
func Lookup(u UID) (Info, bool) {
entry, ok := u.entry()
if !ok {
return Info{}, false
}
info := Info{
UID: u,
Name: entry.Name,
Type: entry.Type,
ExtraInfo: entry.ExtraInfo,
Retired: entry.Retired,
Keyword: entry.Keyword,
}
if entry.Type == "Transfer Syntax" {
info.IsTransferSyntax = true
info.IsCompressed = u.isCompressedTransferSyntax()
info.IsImplicitVR = u == ImplicitVRLittleEndian
info.IsLittleEndian = u != ExplicitVRBigEndian
}
return info, true
}

// LookupKeyword returns the UID that a PS3.6 keyword names, which is the reverse
// of Info.Keyword and the way to reach a UID whose name you have but whose value
// you do not:
//
// u, ok := uid.LookupKeyword("CTImageStorage") // 1.2.840.10008.5.1.4.1.1.2
//
// The generated constants are the better answer when the keyword is known at
// compile time -- uid.CTImageStorage is checked by the compiler and this is not.
// This is for a keyword that arrives at run time, from a configuration file or a
// command line.
func LookupKeyword(keyword string) (UID, bool) {
u, ok := keywordToUID[keyword]
return u, ok
}

Expand Down
Loading
Loading