Improve comment about magic word not present. #318
Conversation
read.go
Outdated
| @@ -160,7 +163,7 @@ func (r *reader) readHeader() ([]*Element, error) { | |||
| return nil, err | |||
| } | |||
| if string(data[128:]) != magicWord { | |||
| return nil, nil | |||
| return nil, ErrorMagicWord | |||
There was a problem hiding this comment.
I think the reason for this was the note at L157: namely, if the magic word isn't found we try to parse the Dataset without the preamble: #59. This was reported by folks who had files like this in production! Perhaps we should make the user pass an explicit parse option to trigger this behavior if we think there's harm in doing this as a default.
Of course, parsing without metadata can cause all sorts of problems which may eventually result in an error anyway.
There was a problem hiding this comment.
@wkoszek do y'all recall if this fix actually lead to y'all successfully parsing DICOMs without the header?
There was a problem hiding this comment.
That makes sense. I reverted the commit, but I updated the comment because it's rather unusual to see a function returning nil, nil like that and the comment was a bit hidden up top. Will update PR title
There was a problem hiding this comment.
Agreed, thanks for the update!
Yep, I think it was indeed intentional: #59 |
This reverts commit 9e8e2f3.
lnogueir
changed the title
lnogueir
changed the title
Just fixing a little bug I stumbled upon while navigating the codebase.Improving comments only.