Skip to content

Commit

Permalink
Use innolitics DICOM Standard JSON dump to generate tags. (#316)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
lnogueir authored May 29, 2024
1 parent a7f7bd4 commit b290e17
Show file tree
Hide file tree
Showing 13 changed files with 8,154 additions and 8,853 deletions.
5 changes: 5 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
BINARY = dicomutil
VERSION = `git describe --tags --always`

.PHONY: codegen
codegen:
- go generate -x ./...
- gofmt -s -w ./pkg/tag

.PHONY: build
build:
go mod download
Expand Down
2 changes: 1 addition & 1 deletion dataset.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ func (d *Dataset) String() string {
tabs := buildTabs(elem.l)
var tagName string
if tagInfo, err := tag.Find(elem.e.Tag); err == nil {
tagName = tagInfo.Name
tagName = tagInfo.Keyword
}

b.WriteString(fmt.Sprintf("%s[\n", tabs))
Expand Down
8 changes: 5 additions & 3 deletions element.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (e *Element) Equals(target *Element) bool {
func (e *Element) String() string {
var tagName string
if tagInfo, err := tag.Find(e.Tag); err == nil {
tagName = tagInfo.Name
tagName = tagInfo.Keyword
}
return fmt.Sprintf("[\n Tag: %s\n Tag Name: %s\n VR: %s\n VR Raw: %s\n VL: %d\n Value: %s\n]\n\n",
e.Tag.String(),
Expand Down Expand Up @@ -147,8 +147,10 @@ func NewElement(t tag.Tag, data interface{}) (*Element, error) {
if err != nil {
return nil, err
}
rawVR := tagInfo.VR

rawVR := tagInfo.VRs[0]
if t == tag.PixelData {
rawVR = "OW"
}
value, err := NewValue(data)
if err != nil {
return nil, err
Expand Down
3 changes: 1 addition & 2 deletions parse_internal_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package dicom

import (
"io/ioutil"
"os"
"strings"
"testing"
Expand All @@ -12,7 +11,7 @@ import (
// This test lives in parse_internal_test.go because this test cannot live in the dicom_test package, due
// to some dependencies on internal valuesets for diffing.
func TestParseUntilEOFConformsToParse(t *testing.T) {
files, err := ioutil.ReadDir("./testdata")
files, err := os.ReadDir("./testdata")
if err != nil {
t.Fatalf("unable to read testdata/: %v", err)
}
Expand Down
16 changes: 8 additions & 8 deletions parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
"encoding/json"
"fmt"
"image/jpeg"
"io/ioutil"
"io"
"os"
"strings"
"testing"
Expand All @@ -20,7 +20,7 @@ import (
// TestParse is an end-to-end sanity check over DICOMs in testdata/. Currently,
// it only checks that no error is returned when parsing the files.
func TestParse(t *testing.T) {
files, err := ioutil.ReadDir("./testdata")
files, err := os.ReadDir("./testdata")
if err != nil {
t.Fatalf("unable to read testdata/: %v", err)
}
Expand All @@ -46,7 +46,7 @@ func TestParse(t *testing.T) {
}

func TestParseUntilEOF(t *testing.T) {
files, err := ioutil.ReadDir("./testdata")
files, err := os.ReadDir("./testdata")
if err != nil {
t.Fatalf("unable to read testdata/: %v", err)
}
Expand Down Expand Up @@ -205,7 +205,7 @@ func BenchmarkParse(b *testing.B) {
}
for _, tc := range cases {
b.Run(tc.name, func(b *testing.B) {
files, err := ioutil.ReadDir("./testdata")
files, err := os.ReadDir("./testdata")
if err != nil {
b.Fatalf("unable to read testdata/: %v", err)
}
Expand All @@ -219,7 +219,7 @@ func BenchmarkParse(b *testing.B) {
}
defer dcm.Close()

data, err := ioutil.ReadAll(dcm)
data, err := io.ReadAll(dcm)
if err != nil {
b.Errorf("Unable to read file into memory for benchmark: %v", err)
}
Expand All @@ -238,7 +238,7 @@ func BenchmarkParse(b *testing.B) {
}

func BenchmarkParser_NextAPI(b *testing.B) {
files, err := ioutil.ReadDir("./testdata")
files, err := os.ReadDir("./testdata")
if err != nil {
b.Fatalf("unable to read testdata/: %v", err)
}
Expand All @@ -252,7 +252,7 @@ func BenchmarkParser_NextAPI(b *testing.B) {
}
defer dcm.Close()

data, err := ioutil.ReadAll(dcm)
data, err := io.ReadAll(dcm)
if err != nil {
b.Errorf("Unable to read file into memory for benchmark: %v", err)
}
Expand Down Expand Up @@ -313,7 +313,7 @@ func Example_getImageFrames() {
}

func runForEveryTestFile(t *testing.T, testFunc func(t *testing.T, filename string)) {
files, err := ioutil.ReadDir("./testdata")
files, err := os.ReadDir("./testdata")
if err != nil {
t.Fatalf("unable to read testdata/: %v", err)
}
Expand Down
5 changes: 2 additions & 3 deletions pkg/dicomio/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"errors"
"fmt"
"io"
"io/ioutil"
"math"

"github.com/suyashkumar/dicom/pkg/charset"
Expand All @@ -21,7 +20,7 @@ var (
)

// LimitReadUntilEOF is a special dicomio.Reader limit indicating that there is no hard limit and the
// Reader should read until EOF.
// Reader should read until EOF.
const LimitReadUntilEOF = -9999

// Reader provides common functionality for reading underlying DICOM data.
Expand Down Expand Up @@ -197,7 +196,7 @@ func (r *reader) Skip(n int64) error {
return ErrorInsufficientBytesLeft
}

_, err := io.CopyN(ioutil.Discard, r, n)
_, err := io.CopyN(io.Discard, r, n)

return err
}
Expand Down
Loading

0 comments on commit b290e17

Please sign in to comment.