Architecture: define a Go-native public API boundary before v1

Architecture proposal: long-term design directions for godicom before v1

I like the current direction of godicom, especially the separation between the core DICOM model, pixel handling, JSON support, and networking through gonetdicom.

Since the project is still pre-v1, I think this may be a good time to discuss some long-term architectural boundaries before the public API becomes difficult to change.

This issue is not proposing an immediate rewrite.

The goal is to describe what I think could be a clean long-term architecture for godicom, and to discuss which parts align with the maintainers' vision.

1. Separate raw parsing, value decoding, and Dataset representation

I think the most important architectural boundary is:

DICOM bytes
    ↓
RawElement
    ↓
VR resolution / value decoding
    ↓
DataElement
    ↓
Dataset

These are conceptually different stages and ideally should remain separate.

A possible internal representation could be:

type RawElement struct {
    Tag Tag

    EncodedVR VR
    Length    uint32

    Offset      int64
    ValueOffset int64

    UndefinedLength bool

    ImplicitVR   bool
    LittleEndian bool

    RawValue RawValue
}

RawElement should represent what was actually encoded in the source, not what the DICOM dictionary says should have been encoded.

For example:

Tag:            (0028,0010) Rows
Dictionary VR:  US
Encoded VR:     SS

The raw layer should preserve SS.

Correction, validation, or tolerant interpretation should happen later.

2. Parser should parse representation, not DICOM semantics

The parser should mainly be responsible for:

Tag
VR
Length
offsets
undefined length
raw value location
item boundaries
sequence boundaries

It should avoid becoming responsible for every DICOM value semantic.

Conceptually:

raw, err := parser.ReadElement()
if err != nil {
    return err
}

elem, err := decoder.DecodeElement(raw, ctx)
if err != nil {
    return err
}

This keeps wire/file parsing and value interpretation independent.

3. Introduce an explicit value codec layer

A decoding context could look conceptually like:

type DecodeContext struct {
    TransferSyntax TransferSyntax
    Charset        CharsetContext
    Dictionary     Dictionary
    Validation     ValidationMode
}

The value layer would handle:

RawElement
    ↓
resolve VR
    ↓
decode primitive representation
    ↓
character set decoding
    ↓
value multiplicity
    ↓
sequence decoding
    ↓
validation
    ↓
DataElement

The reverse path should be handled by a corresponding value encoder.

This would make Reader and Writer share the same DICOM semantic rules instead of developing independent implementations.

4. Keep `DataElement.Value any`, but treat it as a low-level representation

I don't think DICOM values necessarily benefit from forcing everything into generics.

DICOM is inherently heterogeneous.

Something like:

type DataElement struct {
    Tag   Tag
    VR    VR
    Value any
}

is reasonable internally.

However, ideally the allowed concrete value types should be well-defined rather than any meaning literally arbitrary Go values.

For example:

string
[]string

int64
[]int64

uint64
[]uint64

float64
[]float64

[]byte

PersonName
[]PersonName

Date
DateTime
Time

Tag
[]Tag

Sequence
Fragments

This gives the value model a stable contract even if the representation remains dynamic.

5. Typed Dataset APIs should be the primary application interface

Application code should ideally not need to do this frequently:

elem, _ := ds.Get(tag.PatientID)
patientID := elem.Value.(string)

Instead, the normal path should be typed APIs:

patientID, ok := ds.GetString(tag.PatientID)
rows, ok := ds.GetInt(tag.Rows)
seq, ok := ds.GetSequence(tag.ReferencedSeriesSequence)

Writing should have the same symmetry:

ds.SetString(tag.PatientID, "P001")
ds.SetInt(tag.Rows, 512)
ds.SetStrings(tag.ImageType, "ORIGINAL", "PRIMARY")
ds.SetSequence(tag.ReferencedSeriesSequence, seq)

Low-level APIs such as:

ds.GetElement(...)
ds.SetElement(...)

can still exist for advanced use cases.

Conceptually:

normal application
       ↓
typed Dataset API

advanced application
       ↓
DataElement API

library internals
       ↓
RawElement

6. Dataset should remain a DICOM data container, not become a god object

I think Dataset should primarily know about:

elements
sequences
tags
VR values
lookup
set/delete
walking
cloning

It should be difficult for unrelated subsystems to accumulate methods on Dataset.

For example, I would avoid growing Dataset toward:

ds.DecodePixelData(...)
ds.CompressPixelData(...)
ds.ApplyVOI(...)
ds.Window(...)
ds.Level(...)
ds.ToRGB(...)
ds.DecodeJPEG2000(...)

Those operations belong to a separate pixel subsystem.

Keeping Dataset small may become increasingly important as coverage grows.

7. Pixel processing should remain an independent subsystem

Ideally:

px, err := pixels.Open(ds)

frame, err := px.Frame(ctx, 0)

or:

frame, err := pixels.DecodeFrame(ctx, ds, 0)

Transcoding could look like:

err := pixels.Transcode(
    ctx,
    ds,
    uid.JPEG2000Lossless,
)

Dataset provides the necessary DICOM attributes:

Rows
Columns
BitsAllocated
BitsStored
SamplesPerPixel
PhotometricInterpretation
PixelRepresentation
PixelData
TransferSyntax

The pixels subsystem interprets them.

This prevents image-processing concerns from spreading through the core Dataset abstraction.

8. Pixel codecs should be pluggable

Supporting the full DICOM pixel ecosystem eventually means dealing with:

Native
RLE
JPEG
JPEG-LS
JPEG 2000
HTJ2K
MPEG
possibly external/native codecs

The core module should ideally not be forced to depend on every codec implementation.

A codec interface could conceptually look like:

type Decoder interface {
    Supports(ts uid.UID) bool

    Decode(
        ctx context.Context,
        src FrameSource,
        info ImageInfo,
    ) (Frame, error)
}

This allows pure-Go deployments to remain lightweight while applications that need more codecs can opt in.

9. Transfer Syntax should be a domain type, not a raw string

Transfer Syntax is a fundamental DICOM concept.

Instead of APIs requiring values such as:

string(uid.ExplicitVRLittleEndian)

I think it would be cleaner to represent it explicitly:

type TransferSyntax struct {
    UID uid.UID

    ExplicitVR   bool
    LittleEndian bool
    Encapsulated bool
    Deflated     bool
}

Then:

ts := transfer.ExplicitVRLittleEndian

and:

data, err := ds.Encode(ts)

or:

err := godicom.EncodeDataset(w, ds, ts)

Important domain concepts probably should not degrade into generic strings.

10. Charset handling should be an explicit context/subsystem

Character set handling tends to become difficult once ISO 2022 and multi-byte encodings are involved.

Rather than scattering charset conversion throughout the parser, I think it would be useful to have something conceptually like:

type CharsetContext struct {
    Primary    Charset
    Extensions []Charset
}

Then:

func (c CharsetContext) Decode(
    vr VR,
    raw []byte,
) (string, error)

The parsing flow becomes:

(0008,0005) SpecificCharacterSet
              ↓
        CharsetContext
              ↓
       textual VR decoding

Sequence items can inherit the current charset context and update it if necessary.

This keeps text decoding isolated from low-level element parsing.

11. Sequence should have a dedicated recursive codec

SQ parsing is complex enough to deserve its own implementation boundary.

Conceptually:

element parser
      ↓
VR == SQ
      ↓
sequence decoder
      ↓
item decoder
      ↓
Dataset

This would help prevent the main parser from accumulating large amounts of special-case logic for:

defined-length SQ
undefined-length SQ
defined-length Item
undefined-length Item
Item Delimitation Item
Sequence Delimitation Item
nested SQ

Sequence is effectively a recursive Dataset codec and could be treated that way.

12. Streaming should be a first-class capability, not just a Read optimization

Besides:

ds, err := godicom.Read(r)

I think godicom could eventually benefit from a scanner-style API.

For example:

scanner := godicom.NewScanner(r)

for scanner.Next() {
    header := scanner.Header()

    if header.Tag == tag.PixelData {
        scanner.SkipValue()
        continue
    }

    elem, err := scanner.Decode()
    if err != nil {
        return err
    }

    // process element
}

Or perhaps a callback model:

err := godicom.Scan(r, func(e ElementHeader) Action {
    if e.Tag == tag.PixelData {
        return Skip
    }

    if wanted[e.Tag] {
        return Decode
    }

    return Skip
})

This would be especially useful for PACS indexing workloads.

Extracting:

PatientID
StudyInstanceUID
SeriesInstanceUID
SOPInstanceUID
StudyDate
Modality
AccessionNumber

should ideally not require materializing hundreds of MB of Pixel Data.

13. Deferred values should depend on an abstract source, not necessarily a filename

Deferred reading could be more flexible if backed by an abstraction such as:

type ValueSource interface {
    ReadAt(p []byte, off int64) (int, error)
}

Then:

type DeferredValue struct {
    Source ValueSource
    Offset int64
    Length int64
}

This makes deferred values usable with:

os.File
memory-backed sources
mmap
cached object storage
custom seekable sources

rather than tying deferred loading directly to local file paths.

14. Reader and Writer should share the same codec model

Ideally decoding and encoding are symmetric:

decode:

bytes
  ↓
RawElement
  ↓
ValueDecoder
  ↓
DataElement
  ↓
Dataset


encode:

Dataset
  ↓
DataElement
  ↓
ValueEncoder
  ↓
RawElement
  ↓
bytes

This reduces the risk that:

Reader accepts one representation
Writer produces another

because the two sides accidentally implement DICOM VR semantics differently.

15. Support explicit Strict / Warn / Tolerant validation modes

Real-world DICOM often contains data that is technically incorrect but still needs to be processed.

I think validation policy should eventually be explicit:

type ValidationMode int

const (
    Strict ValidationMode = iota
    Warn
    Tolerant
)

For example:

Dictionary VR: US
Encoded VR:    SS

could produce:

Strict

return error

Warn

record diagnostic
attempt best-effort decoding

Tolerant

best-effort decoding

This behavior should ideally be controlled by policy rather than hard-coded throughout the parser.

16. Diagnostics should preserve structural information

It would be useful for parser/decoder warnings to preserve context such as:

type Diagnostic struct {
    Tag      Tag
    Offset   int64

    Expected VR
    Actual   VR

    Message string
}

Potentially also:

sequence/item path
value length
transfer syntax
validation severity

For example:

(0040,0100)[0]
  └── (0008,0104)

This kind of diagnostic information is extremely useful when debugging interoperability problems with real DICOM devices.

17. Dictionary should be represented by an interface

Rather than coupling decoding permanently to one global dictionary, a small abstraction could be useful:

type Dictionary interface {
    Lookup(tag Tag) (Entry, bool)
}

Then the default can remain:

dictionary.Standard()

while applications may compose:

dictionary.New(
    dictionary.Standard(),
    dictionary.Private(...),
)

This provides a clean path for:

standard tags
repeater tags
private dictionaries
vendor-specific dictionaries
testing dictionaries

without modifying the global standard dictionary.

18. Private Creator / Private Block should be first-class concepts

Private elements are not simply ordinary unknown tags.

It may be useful to model private blocks explicitly.

For example:

block, err := ds.PrivateBlock(
    0x0019,
    "GEMS_ACQU_01",
)

elem, ok := block.Get(0x10)

instead of requiring application code to manually understand private creator block allocation.

This could make vendor-specific metadata significantly easier and safer to work with.

19. Keep `Dataset` and Part 10 `FileDataset` strictly distinct

I think this distinction is important and worth preserving.

Dataset should represent a generic DICOM Dataset that may come from:

DIMSE
DICOM Part 10 file
DICOMweb
memory
tests
other transports

while FileDataset should specifically represent the PS3.10 file container:

type FileDataset struct {
    Dataset *Dataset
    FileMeta FileMeta
    Preamble [128]byte
}

DIMSE identifiers such as C-FIND datasets should never need to pretend to be DICOM files.

This also helps keep file-level concerns such as preamble and File Meta Information outside the core Dataset abstraction.

20. Keep networking and DICOMweb transport outside the core dependency direction

The dependency direction should ideally remain:

gonetdicom
    ↓
godicom

and never:

godicom
    ↓
gonetdicom

The godicom core should not need to understand:

Association
Presentation Context
DIMSE commands
C-FIND
C-MOVE
C-STORE state machines
SOP negotiation
network transport

Likewise, DICOM JSON support can reasonably live close to Dataset because it represents PS3.18's Dataset JSON model, but HTTP-level concepts such as:

QIDO-RS
WADO-RS
STOW-RS
multipart HTTP
routing
HTTP authentication

should ideally remain in a higher-level DICOMweb package.

Public API philosophy

More generally, I think it would be valuable to intentionally keep the root package small.

Ideally the most common API remains something close to:

godicom.Read(...)
godicom.Write(...)

godicom.NewDataset()

type Dataset
type DataElement
type Tag
type VR

while specialized concepts live in focused packages:

tag.PatientName
uid.CTImageStorage

pixels.DecodeFrame(...)
dicomjson.Marshal(...)
dictionary.Lookup(...)
charset.Decode(...)

The root package should probably resist gradually becoming:

godicom.DecodeJPEG2000
godicom.GenerateUID
godicom.ApplyVOI
godicom.ParseDate
godicom.PrivateTagLookup
godicom.HTTPGet
godicom.Retry
...

Suggested architecture

Putting these ideas together, the long-term architecture I have in mind looks roughly like this:

                         Application
                              │
                ┌─────────────┼─────────────┐
                │             │             │
              PACS        gonetdicom     DICOMweb
                │             │             │
                └─────────────┴─────────────┘
                              │
                          Dataset
                              │
                    Typed Dataset API
                              │
                         DataElement
                              │
                      DICOM Value Model
                              │
                   Value Decoder / Encoder
                              │
                          RawElement
                              │
                     Parser / Raw Writer
                              │
                    io.Reader / io.Writer


        Independent specialized subsystems

        ┌────────────┐
        │   pixels   │
        ├────────────┤
        │ frame      │
        │ native     │
        │ encaps     │
        │ codecs     │
        │ color      │
        │ LUT / VOI  │
        └────────────┘

        ┌────────────┐
        │  charset   │
        ├────────────┤
        │ UTF-8      │
        │ ISO 2022   │
        │ GB18030    │
        │ JIS        │
        │ Korean     │
        └────────────┘

        ┌────────────┐
        │ dictionary │
        ├────────────┤
        │ standard   │
        │ repeater   │
        │ private    │
        └────────────┘

Possible architecture invariants before v1

Perhaps some of these could eventually be written down as project-level architecture rules:

Dataset does not depend on DIMSE/networking.
Dataset is primarily a DICOM data container, not an image-processing object.
Pixel codecs remain independent from the core Dataset implementation.
Raw DICOM representation and decoded values remain separate concepts.
Parser responsibilities and DICOM value semantics remain separated.
Reader and Writer share the same VR/value codec rules.
Streaming and deferred reading remain possible without loading full values.
Character set handling remains an isolated subsystem.
Public APIs prefer typed Dataset operations over direct any assertions.
Important DICOM concepts use domain types rather than generic strings where practical.
Private dictionaries can be extended without modifying the standard dictionary.
pydicom may be used as a behavioral/correctness oracle without requiring API-level parity.

Summary

The main idea is not to add more abstractions for their own sake.

The goal is to keep difficult concerns independent:

representation
     ↓
interpretation
     ↓
domain model
     ↓
specialized processing

DICOM itself is already extremely complicated.

If these boundaries stay clear, I think godicom could grow significantly in standards coverage and real-device compatibility without making the core architecture significantly more complicated.

I'd be interested to hear which of these directions align with the long-term design goals for godicom, and which ones the maintainers intentionally want to approach differently.

Status after v0.27.0

Working through this in phases, one PR each. v0.27.0 is the first tag that carries any of it, so here is what landed against which section and what deliberately did not.

Landed

§13 — deferred values backed by an abstract source. This is the section that got the most work, because it was also a bug. readContext now holds an io.ReaderAt rather than only a path, which is the ValueSource shape §13 proposed (internal, not a public DeferredValue).

Fix unreachable deferred values for non-file seekable readers #52 — deferred values were permanently unreachable after Read from a seekable reader that is not an *os.File. The tag stayed listed while Get reported it absent, so bytes.Reader, io.SectionReader and range-request adapters silently lost every deferred element.
Point the deferred source at the inflated bytes of a deflated dataset #57 — in a Deflated dataset the deferred source pointed at the still-compressed buffer, so loads failed with a tag or VR mismatch. It now points at the inflated bytes.
Read a sized io.ReaderAt without the seek wrapper. #59 — a reader that already offers io.ReaderAt plus Size() int64 is used directly instead of being wrapped in Seek+Read, so neither the parse nor a later deferred load moves the caller's offset. mmap-style and object-storage sources are the cases this matters for.

§5 — typed Dataset APIs as the primary interface. #53 added the write half: SetString / SetInt / SetFloat / SetBytes / SetSequence, the plural forms, and SetDA / SetTM / SetDT / SetPN / SetDS / SetIS. Each takes the VR from the dictionary and rejects value kinds that VR cannot hold, so the symmetry you sketched (GetString ↔ SetString) now holds without the caller naming a VR. Tags outside the dictionary still need Set(NewDataElement(tag, vr, value)).

§16 — diagnostics that preserve structural information. #54 added ReadOptions.OnDiagnostic and a Diagnostic carrying tag, VR, byte offset, the enclosing sequence path, and how many bytes were needed versus present. It is close to your struct, with the sequence/item path included.

Return errors from the sequence readers instead of latching them #56 — diagnostics raised inside a sequence, and hook errors rejecting them, were being swallowed by the sequence readers. They now reach the caller.
Report a defined-length sequence that overruns the file. #58 — the streaming reader silently dropped a defined-length sequence whose declared length overran the file, complete items included. It now parses the items that are present and reports a truncated item, agreeing with ReadBytes and with pydicom's read_sequence_item.

§15 — validation policy, partially. OnDiagnostic gives strict (return the diagnostic; Diagnostic is an error) and tolerant (return nil) as a caller decision rather than parser-hardcoded behaviour. There is no ValidationMode enum and no Warn tier — the hook covers Warn by recording and returning nil.

§9 — transfer syntax, partially. #53 changed every transfer syntax parameter and return from string to uid.UID: EncodeDataset, WriteDataset, Dataset.Encode, DecodeDataset, DecodeDatasetContext, CompressPixelData, CompressPixelDataContext, FileDataset.TransferSyntaxUID, pixels.FileSource. string(uid.ExplicitVRLittleEndian) is gone from the call sites you flagged. This is not the TransferSyntax struct from §9 — see below.

§2 — one decoder for the representation. #55 routes every data element header, streaming and in-memory, through a single decodeElementHeader. Two independent implementations had drifted on the malformed cases; #58's regression test asserts the two readers now report an identical diagnostic for the same bytes. A step toward §2's boundary rather than the boundary itself.

Also from #53, found while adding the setters: AT elements were encoded as one 32-bit value instead of the group/element pair PS3.5 7.1.1 requires. The encodings coincide under big endian and differ under little endian, so every AT value godicom wrote under the default transfer syntax read back with group and element swapped — (0018,1063) as (1063,0018) — by godicom and by everything else. That fix is the reason v0.27.0 is tagged now rather than left on main.

Not started, on purpose

§3 (DecodeContext), §10 (CharsetContext), §14 (reader/writer sharing one codec model), §1 (RawElement). Each of these means introducing a public type that becomes part of the API surface, and the project rule is that new public API is confirmed with the maintainer first rather than invented by whoever is refactoring. So they are not mine to decide. §14 in particular is the one I would expect to pay off most — the read paths were unified with each other in #55, not with the writer, and that is where reader/writer VR divergence would show up.

The pixel and dependency-direction sections (§7, §8, §20) already match how the tree is laid out; §19 likewise, Dataset and FileDataset are distinct today. §6 is a judgement call about which existing methods to move and would be a breaking change worth batching with whatever else breaks.

Leaving this open — the sections above are the ones that need a decision from you before anything else lands.

Format a DS float the way the DS type already does. (#62)

Refuse a NaN or an infinity in a DS instead of writing it out. (#64)

§14 (reader and writer sharing one value model) — investigated, and it looks closeable

Rather than restructure on the strength of the section's argument, I went looking for behaviour that actually diverges.

Method. For all 33 VRs plus their multi-value forms: representative bytes → convertValueWithCharsets → encodeValue → compare against the input. Everything came back byte-identical except text VRs losing a trailing pad byte, which turned out to be the probe bypassing padToEven — verified separately that the real write boundary re-pads.

Found two real divergences, both DS formatting, both fixed:

Format a DS float the way the DS type already does. #62 — the writer rendered a bare float64 in a DS with %g, which has no length bound, while PS3.5 caps DS at 16 bytes. SetFloat(SliceThickness, 1.0/3.0) wrote 18 bytes that IsValidDS rejects. FormatNumberAsDS had known the rules since v0.15.0; the writer had reimplemented them next door. Reachable through the public setters because DS is a float VR, so SetFloat stores a plain float64.
Refuse a NaN or an infinity in a DS instead of writing it out. #64 — NaN / ±Inf went into a DS as the literal bytes "NaN" / "+Inf" / "-Inf" with no error from the setter, the encode, or the save. encodeValue now returns an error so the writer can refuse them, and the setters reject them at the call site. Scoped to DS: FD/FL represent all three exactly per IEEE 754 and are unchanged.

Both went unnoticed for the same reason: ParseDS is tolerant, so godicom read its own invalid output back without complaint, and every byte-identical fixture test exercises the DS-typed branches rather than the bare-float64 one.

Conclusion. The reader and writer already share the value types — encodeStringWithCharsets handles DA/TM/DT/DS/IS/PersonName and their multi-value forms directly, and the numeric and binary paths line up across every width. §14's "separate value models" showed up as behaviour in exactly two places, both now fixed, so I don't think the restructuring it proposes buys anything. Suggest closing §14 unless there's a divergence I didn't probe.

One new open API question, added to the list below: a fractional float in an IS still formats as "1.5", which is not a valid integer string. Refusing it and rounding it are both defensible and they differ in what callers can express, so per AGENTS.md that needs a human decision rather than a bug fix. (A non-finite float in an IS is refused in #64, since there's no ambiguity there.)

Status

§	Topic	State
§2	AT encoding	done (#61 series)
§5	shared header decoder	done
§9	transfer syntax as `uid.UID`	done for the typed params; the `TransferSyntax` struct is new public API — not started
§13	diagnostics hook	done
§14	reader/writer value model	investigated; two fixes landed (#62, #64); proposed restructuring looks unnecessary
§15	sequence readers return errors	done
§16	deferred value sources	done
§1	`RawElement`	not started — new public API
§3	`DecodeContext`	not started — new public API
§10	`CharsetContext`	not started — new public API
new	fractional float in `IS`	open — API question

The four "not started" rows are all new public API or a breaking change, and AGENTS.md says to confirm the API with a human before inventing it, so I'm leaving them for you rather than deciding unilaterally. Everything landed so far has been internal.

Progress report on the incremental part of this proposal, plus a recorded
position on three sections I do not think godicom should implement as written.

Landed

Six PRs, each its own phase, all merged with CI green:

PR	what it settles here
#62	a `DS` held as a plain `float64` is now written the way the `DS` type already spells it, so one value has one encoding regardless of which Go type holds it
#63	removed two dead `pos` assignments the readers never read back
#64	`NaN` / `±Inf` are refused for a `DS` instead of being written as the literal bytes `"NaN"`
#65	§14 (write side): the encoding triple `(isImplicitVR, isLittleEndian, charsets)` threaded through the write chain is now one `codecContext`
#66	§14 (read side): the same for the read chain, so both sides carry the identical struct
#67	§15: `WriteOptions.OnDiagnostic`, mirroring `ReadOptions.OnDiagnostic`

§14 is done for the encoding state. Reader and writer now pass the same
codecContext (an embedded EncodingInfo plus Charsets) rather than three
loose parameters each side spelled its own way. The granularity rule is
deliberate: a function that needs only the encoding pair takes EncodingInfo, a
function that also touches text takes the full codecContext, and an
endianness-only helper keeps a bare bool. That is what stops the "reader
accepts one representation, writer produces another" drift the section is
about — the two sides can no longer disagree about what they were told, because
they are told by the same type.

§15 — satisfied, without the enum

The section asks for Strict / Warn / Tolerant as a policy value. #67 gives
the behaviour without adding a mode:

// Strict
opts := &WriteOptions{OnDiagnostic: func(d Diagnostic) error { return d }}

// Warn
opts := &WriteOptions{OnDiagnostic: func(d Diagnostic) error { log.Print(d); return nil }}

// Tolerant — the default, no hook
opts := &WriteOptions{}

Whether the hook is set, and what it returns, already carries three states. An
enum on top would be a second way to say the same thing, and the two could
disagree (Tolerant + a hook that returns an error — which wins?). pydicom
reaches the same three behaviours through
config.settings.writing_validation_mode; godicom reaches them through the hook
it already had on the read side. The hook is also strictly more expressive: the
caller can be strict about one tag and tolerant about another, which an enum
cannot express.

§1 — already true, and the type already exists

The raw layer should preserve SS.

It does, and it did before this work. decodeElementHeader (header.go:35)
returns the VR read off the wire and consults the dictionary only when the
wire carried no usable VR — implicit VR, or explicit VR whose two bytes are not
uppercase letters (pydicom issues 1067 and 1035):

if ExplicitVRLength16[vr] {
    ...
    return elementHeader{VR: vr, Length: length, Size: 8}, 8, true   // encoded VR, not LookupVR(tag)
}

A (0028,0010) encoded as SS is decoded as SS. The RawElement type the
section proposes also exists as RawDataElement (element.go:229), carrying
Tag, VR, Length, Value, ValueTell, IsImplicitVR, IsLittleEndian.
Field names differ from the sketch and it has no separate Offset, but the
boundary the section asks for is there.

So I have not changed anything for §1. Renaming the existing type to match the
sketch would be churn against a public identifier for no behavioural gain.

§9 — declining: it would create a second source of truth

The proposed struct duplicates state uid.UID already derives:

type TransferSyntax struct {
    UID          uid.UID
    ExplicitVR   bool   // uid.UID.IsImplicitVR()
    LittleEndian bool   // uid.UID.IsLittleEndian()
    Encapsulated bool   // uid.UID.IsEncapsulated()
    Deflated     bool   // uid.UID.IsDeflated()
}

uid/uid.go already has all of them as methods on the UID, plus
IsCompressed(), IsTransferSyntax() and IsValid(). The public API already
takes uid.UID, not string — the section's string(uid.ExplicitVRLittleEndian)
example is not what godicom asks for today.

Making the flags struct fields makes them settable, which means
TransferSyntax{UID: uid.ImplicitVRLittleEndian, ExplicitVR: true} is
constructible and self-contradictory, and every consumer then has to decide
whether to trust the UID or the flag. As methods there is exactly one answer,
derived from the UID, and it cannot be desynchronised. I would rather add a
method to uid.UID than a struct that can lie about it.

If the real complaint is ergonomic — that uid.ExplicitVRLittleEndian is a
mouthful, or that a caller wants the flags without knowing which method to
call — that is worth fixing, and I would take a concrete example of a call site
that reads badly today.

Not addressed

Everything else in the proposal is untouched: §2–§8, §10–§13, §16–§20. Several
are large enough to want their own issue rather than a phase of this one,
particularly §12 (streaming as a first-class capability) and §17 (dictionary as
an interface). The rule I have been following throughout is the repo's own —
no new public API where an existing one can carry the meaning — which is why
four of the six PRs above added no exported identifier at all.

Name which item of a sequence a diagnostic came from. (#69)

Split out the two sections I said were large enough to want their own issue:

Streaming should be a first-class capability (#51 §12) #72 — §12, streaming as a first-class capability
Dictionary should be represented by an interface (#51 §17) #71 — §17, dictionary as an interface

Both write down where godicom stands today and the design questions that have to
be answered before any code moves, rather than jumping to an API.

Also shipped in v0.28.0, closing out §16 (diagnostics should preserve structural
information):

Report values the writer would otherwise encode silently. #67 — the writer reports a value it would otherwise encode silently
(DiagnosticInvalidValue)
Report a VR the data dictionary cannot reconcile with its tag. #68 — reading reports a VR the data dictionary cannot reconcile with its tag
(DiagnosticVRMismatch, plus Diagnostic.ExpectedVR)
Name which item of a sequence a diagnostic came from. #69 — Diagnostic.Path names which item of each enclosing sequence the
anomaly came from, not merely which sequence ([]Tag → []PathStep)

and §14's groundwork, the read and write chains now carrying the encoding triple
as one codecContext instead of three parameters threaded through fourteen
signatures (#65, #66).

What §16 still leaves open, if you want it: transfer syntax and a validation
severity on Diagnostic. Severity is really §15's question (Strict / Warn /
Tolerant modes), so I would rather do it there than bolt a field on now.

Ask for a private block by vendor name, and let a caller create one. (#…

Closing

The sections that were going to change the public API have either landed or been declined. Nothing in this issue is still waiting on a design decision.

Landed

§5 typed setters (SetString, SetInt, SetFloat, and the date / time / PN / DS / IS forms) in v0.27.0
§9 transfer-syntax parameters take uid.UID in v0.27.0
§13 deferred values load through io.ReaderAt in v0.27.0
§14 read and write share one codecContext. A separate public value-codec type was not added. The behaviour gaps were the DS length and non-finite DS fixes (Format a DS float the way the DS type already does. #62, Refuse a NaN or an infinity in a DS instead of writing it out. #64)
§15 Strict / Warn / Tolerant is the diagnostic hook: return the error, log and return nil, or leave the hook unset. There is no ValidationMode enum
§16 ReadOptions.OnDiagnostic and WriteOptions.OnDiagnostic, carrying tag, VR, offset, and the sequence path ([]PathStep)
§17 Dictionary, Standard, NewDictionary, NewPrivateDictionary, and ReadOptions.Dictionary in v0.30.0 (Dictionary should be represented by an interface (#51 §17) #71)
§18 PrivateBlock, NewPrivateBlock, and PrivateCreators in v0.30.0
§12 ReadOptions.OnElement (ElementKeep / ElementSkip / ElementStop) is on main (Streaming should be a first-class capability (#51 §12) #72, Let a caller decide, per element, whether to read the value at all #86) and is not in a tag yet
§19 Dataset and FileDataset stay distinct
§20 DIMSE and DICOMweb stay in gonetdicom

Declined

§1 renaming RawDataElement. When the wire carries a VR, that is the VR the reader keeps
§3 DecodeContext and §10 CharsetContext as new public types. ReadOptions and the charset path already carry that state
§9 a TransferSyntax struct with settable flags. The flags are methods on uid.UID, so they cannot disagree with the UID
§14 a separate public encode/decode type. One codecContext is the shared state

Left as they already were

§6–§8 pixel work lives in pixels. FileDataset keeps PixelArray, DisplayFrame, and CompressPixelData as the entry points
§2 one element-header decoder serves both the streaming and the in-memory readers
§11 sequence parsing stays inside the reader. There is no separate public sequence codec

A fractional float64 stored on an IS with Set is still written as "1.5". SetFloat rejects an IS tag. The writer reports the value through OnDiagnostic and does not round. TestWriteReportsFractionalIS and TestISFloatFormattingUnchanged lock that in.

Architecture: define a Go-native public API boundary before v1 #51

Description

Architecture proposal: long-term design directions for godicom before v1

1. Separate raw parsing, value decoding, and Dataset representation

2. Parser should parse representation, not DICOM semantics

3. Introduce an explicit value codec layer

4. Keep DataElement.Value any, but treat it as a low-level representation

5. Typed Dataset APIs should be the primary application interface

6. Dataset should remain a DICOM data container, not become a god object

7. Pixel processing should remain an independent subsystem

8. Pixel codecs should be pluggable

9. Transfer Syntax should be a domain type, not a raw string

10. Charset handling should be an explicit context/subsystem

11. Sequence should have a dedicated recursive codec

12. Streaming should be a first-class capability, not just a Read optimization

13. Deferred values should depend on an abstract source, not necessarily a filename

14. Reader and Writer should share the same codec model

15. Support explicit Strict / Warn / Tolerant validation modes

Strict

Warn

Tolerant

16. Diagnostics should preserve structural information

17. Dictionary should be represented by an interface

18. Private Creator / Private Block should be first-class concepts

19. Keep Dataset and Part 10 FileDataset strictly distinct

20. Keep networking and DICOMweb transport outside the core dependency direction

Public API philosophy

Suggested architecture

Possible architecture invariants before v1

Summary

Activity

xbmlz commented on Aug 20, 2026

Status after v0.27.0

Landed

Not started, on purpose

xbmlz commented on Aug 21, 2026

§14 (reader and writer sharing one value model) — investigated, and it looks closeable

Status

xbmlz commented on Aug 21, 2026

Landed

§15 — satisfied, without the enum

§1 — already true, and the type already exists

§9 — declining: it would create a second source of truth

Not addressed

xbmlz commented on Aug 24, 2026

xbmlz commented on Oct 20, 2026

Closing

Landed

Declined

Left as they already were

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

4. Keep `DataElement.Value any`, but treat it as a low-level representation

19. Keep `Dataset` and Part 10 `FileDataset` strictly distinct