XAM.isproperpair() fix

[hanningk]

Use of XAM.isproperpair(record) returns:

UndefVarError: `PROPER_PAIR` not defined in `XAM`

I think the function should be using FLAG_PROPER_PAIR instead.

This PR implements the following changes:

• 🐛 Bug fix (A non-breaking change, which fixes an issue).

[kescobo]

This doesn't really make any sense (the previous version didn't either to be clear). The right hand expression will always return true or throw an error. Is it supposed to be FLAG_PROPER_PAIR == PROPER_PAIR or something? That wouldn't fix the error you're seeing but would at least be sensible.

What am I missing here?

[hanningk]

Apologies in advance - I'm new to this.

I figured this was a simple oversight as the other FLAG query functions contain the FLAG_ prefix for their constants. They are defined in the loop starting at line 33 in src/flags.jl

An example for a different FLAG:

function ispaired(record::XAMRecord)::Bool
    return flags(record) & FLAG_PAIRED == FLAG_PAIRED
end

Shouldn't the logic be:

const FLAG_A = UInt8(0x01)      # 00000001
const FLAG_B = UInt8(0x02)      # 00000010
const FLAG_C = UInt8(0x04)      # 00000100

flag_record = FLAG_A | FLAG_C   # 00000101

flag_record & FLAG_A == FLAG_A  # true
flag_record & FLAG_B == FLAG_B  # false 
flag_record & FLAG_C == FLAG_C  # true

I'll look more into how XAM records handle the flag field.

[MillironX]

@kescobo The expression breaks down into

return (
    flags(record)    # The integer with the set flags of this record
    &                # bitwise AND
    FLAG_PROPER_PAIR # the constant of the PROPER_PAIR flag (0x002)
    )                # Will be equal to PROPER_PAIR (0x002) if the flag was set in the
    ==               #     record, otherwise won't be
    FLAG_PROPER_PAIR

I think you were mistaking the bitwise AND for logical AND ALSO.

Page 7 of the SAM specification has a pretty detailed explanation and plenty of examples.

@hanningk Thanks for this patch! Good catch! lgtm. Tho I am wondering why we didn't have a test for this in hindsight. 🤔

[kescobo]

Ahh, I see - you're right I didn't realize the precedence was different.

[MillironX]

@kescobo The expression breaks down into

return (
    flags(record)    # The integer with the set flags of this record
    &                # bitwise AND
    FLAG_PROPER_PAIR # the constant of the PROPER_PAIR flag (0x002)
    )                # Will be equal to PROPER_PAIR (0x002) if the flag was set in the
    ==               #     record, otherwise won't be
    FLAG_PROPER_PAIR

I think you were mistaking the bitwise AND for logical AND ALSO.

Page 7 of the SAM specification has a pretty detailed explanation and plenty of examples.

@hanningk Thanks for this patch! Good catch! lgtm. Tho I am wondering why we didn't have a test for this in hindsight. 🤔

[kescobo]

Tho I am wondering why we didn't have a test for this in hindsight. 🤔

Should we add one now?

[CiaranOMara]

Thank you @hanningk. Your fix was release in v0.4.1