Inconsistent BAM flag accessor functions

[jakobnissen]

Inconsistent BAM flag accessor functions

Currently, it's possible to create a spec-compliant BAM record with the following behaviour:

julia> BAM.hasrefname(record)
true

julia> BAM.refname(record)
ERROR: ArgumentError: record is not mapped
Stacktrace:
 [1] checked_refid at /home/jakni/.julia/packages/BioAlignments/AGQde/src/bam/record.jl:162 [inlined]
 [2] refname(::BioAlignments.BAM.Record) at /home/jakni/.julia/packages/BioAlignments/AGQde/src/bam/record.jl:178
 [3] top-level scope at none:0

(on a related note, the function BAM.hasnextrefname simply doesn't work.)

What's going on?

Well, for a subset (which subset, I'm not quite sure) of the flags, there is a corresponding flag accessor isX function and a function hasX. I'm not entirely sure what the purpose of the hasX function are - some of them seem to be exported to BioCore. It might be I'm simply confused as to what the purpose of these functions are, if so, please tell me.
Anyway, the behaviour of these functions are inconsistent and highly unintuitive. For example, ismapped, isprimary and ispositivestrand all raise an error if the record is unfilled, else return a boolean. However isnextmapped returns false with an unfilled record.

Furthermore, hasrefid, hasrefname, hasnextrefid returns whether the record is filled, NOT whether the record is aligned (i.e. actually has a refid etc.). So a valid, unmapped record can have an ID of -1 and still return true when asked if hasrefid. The similar function hasnextrefname, however, actually does check whether it has a valid next reference name.

Also, refname and nextrefname checks for the validity of the corresponding IDs before fetching the name, whereas hasrefname does not (hence the error at the beginning of this issue).

hasposition returns true if the Record is unmapped (i.e. has an invalid position), but hasrightposition returns false in that case.

hasflag seem to me to be meaningless. All BAM records by definition has flags. Currently, it simply returns isfilled(record), and is not used in any other code.

This seems very unsystematic. I propose changing all these functions to follow the following rules:

• If an existing function hasX returns true, then isX will always return a boolean which is guaranteed to be correct.
• If an existing function hasX returns false, then isX will always throw an error
• The functions hasrefid, hasrefname, hasnextrefid and hasnextrefname will return false if the record is unfilled, or if the record/mate is unmapped, else true
• The functions hasposition, hasnextposition and hasrightposition returns false is the record is unfilled or the record/mate is unmapped, else true

[TransGirlCodes]

@jakobnissen Did this get fixed to your satisfaction in your PR? I'm trying to figure if this can be closed or if it's still troubling you?

[jakobnissen]

So it turns out it's an operation a little larger than I expected - and I've been short on time due to work the past month or so. Feel free to close this issue or not; I'll create a PR some time when I have the time either way :)