Read CSI region #48
Read CSI region #48
Conversation
internal/csi/csi.go
Outdated
| @@ -71,3 +82,73 @@ func BinsForRange(start, end uint32, minShift, depth int32) []uint16 { | |||
| func maximumBinWidth(minShift, depth int32) uint32 { | |||
| return uint32(1 << uint32(minShift+depth*3)) | |||
| } | |||
|
|
|||
| // Read reads index data from csi and returns a set of BGZF chunks covering | |||
There was a problem hiding this comment.
@anamanolache This is hard to read. Just say Reads a CSI formatted index file.
internal/csi/csi.go
Outdated
| @@ -71,3 +82,73 @@ func BinsForRange(start, end uint32, minShift, depth int32) []uint16 { | |||
| func maximumBinWidth(minShift, depth int32) uint32 { | |||
| return uint32(1 << uint32(minShift+depth*3)) | |||
| } | |||
|
|
|||
| // Read reads index data from csi and returns a set of BGZF chunks covering | |||
There was a problem hiding this comment.
csiFile -> csi (it's a Reader, not a file)
Or just 'r' is fine too
internal/csi/csi.go
Outdated
|
|
||
| var minShift int32 | ||
| if err := binary.Read(gzr, &minShift); err != nil { | ||
| return nil, fmt.Errorf("reading # bits for the minimal interval (min_shift): %v", err) |
There was a problem hiding this comment.
"reading minimum interval width: %v"
There was a problem hiding this comment.
N/A, refactored code
internal/csi/csi.go
Outdated
| return nil, fmt.Errorf("checking magic: %v", err) | ||
| } | ||
|
|
||
| var minShift int32 |
There was a problem hiding this comment.
minShift is a detail of the implementation - it's best to talk about widths outside the implementation of the function
internal/csi/csi.go
Outdated
| return nil, fmt.Errorf("checking magic: %v", err) | ||
| } | ||
|
|
||
| var minShift int32 |
There was a problem hiding this comment.
I'd typically suggest you use a structure and read:
var header struct {
minimumWidth int32
depth int32
auxilaryLength int32
}
if err := binary.Read(gzr, &header) {
...
internal/csi/csi.go
Outdated
| if err := binary.Read(gzr, &depth); err != nil { | ||
| return nil, fmt.Errorf("reading depth of binary index: %v", err) | ||
| } | ||
| bins := BinsForRange(region.Start, region.End, minShift, depth) |
internal/csi/csi.go
Outdated
| @@ -71,3 +82,68 @@ func BinsForRange(start, end uint32, minShift, depth int32) []uint16 { | |||
| func maximumBinWidth(minShift, depth int32) uint32 { | |||
| return uint32(1 << uint32(minShift+depth*3)) | |||
| } | |||
|
|
|||
| // Read reads index data from CSI and returns a set of BGZF chunks covering | |||
There was a problem hiding this comment.
Read reads CSI formatted index data from r and returns a set of ...
internal/csi/csi.go
Outdated
| if err := binary.Read(csi, &refCount); err != nil { | ||
| return nil, fmt.Errorf("reading the number of reference sequences: %v", err) | ||
| } | ||
| bins := BinsForRange(region.Start, region.End, csiHeader.MinimumWidth, csiHeader.Depth) |
There was a problem hiding this comment.
Ideally we'd factor out the common code between here and the BAM reader. The difficulty is the Offset field. It's OK to submit this now, but maybe file an issue about it. We could have some common function that takes a flag, for example, that would cover both.
There was a problem hiding this comment.
I've done it now so I don't commit duplicate code to begin with.
No description provided.