feat: add bam::flag module for easy handling of SAM/BAM flags

[rLannes]

Description:

This pull request introduces a new module, bam::sam_flag, to facilitate easier handling of SAM flags. The module provides a more intuitive and type-safe way to work with SAM flags, improving code readability and reducing potential errors.

Key features:

1. SamFlag struct: A zero-cost abstraction that contains constants for all standard SAM flags.
2. check_flag() function: A utility function to easily check for the presence or absence of specific flags.

Benefits:

• Improved code readability when working with SAM flags.
• Reduced chance of errors from manual flag manipulation.
• Type-safe constants for all standard SAM flags.
• Easy-to-use function for checking flag combinations.

Implementation choice:
The main design choice was to use a struct with associated constants rather than an enum to represent the flag values. The benefit is no runtime cost, but it comes at the cost of not being as nice to work with as an enum.

Example usage:

let read_flag = 65; // PAIRED (1) + FIRST_IN_PAIR (64)
assert!(check_flag(read_flag, SamFlag::PAIRED + SamFlag::FIRST_IN_PAIR, SamFlag::READ_UNMAPPED));

[johanneskoester]

Nice!

[johanneskoester]

I think I would prefer this to be called simply flags, and the struct Flag instead of SamFlag.

[rLannes]

Thank you for your return! I changed the name of both the module and the Structure. And ran cargo fmt. (Sorry for that, it is the first time I am contributing to a rust repo).
Romain,

[johanneskoester]

Isn't the sam_flag module just a repetition of flags now? Did you mean to remove it?
Also, would it make sense to have two separate methods is_in() and is_not_in() for the Flags struct instead of check_flags?

[rLannes]

Thank you for your return.

1- I am not sure what you are referring to with this: "Isn't the sam_flag module just a repetition of flags now? Did you mean to remove it?". I thought I did by changing the sam_flag module name to flags, as suggested. Is that not the case?

As of the latest commit to use this module, one has to import: use rust_htslib::bam::flags; Did I miss something, or fail to update/commit??

2- I often have to test for both the presence and absence of a flag, but now that you mentioned it, it is clearly confusing for an API. I will split this function into two and change the doc string accordingly.

Because a Flag is a struct with an associated constant, I can't write methods like flag_instance.is_in(value), but rather is_in(Flag::PAIRED + Flag::PROPERLY_PAIRED, value); (Flag(s) will be interpreted as an u16.)
Does it make sense then to call the functions: is_in_flag(flag_value, value) and is_not_in_flag(flag_value, value)?

Thank you for your time and suggestion!

[rLannes]

I added two functions (removed the old one) and updated the docstring accordingly.
You now use them like this:

use rust_htslib::bam::flags{Flag, is_in_flag, is_not_in_flag};
//let read_flag = record.flag(); // In general, this is the way to obtain a flag.
let read_flag = 64;
assert_eq!(is_in_flag(read_flag, Flag::FIRST_IN_PAIR), true);
assert_eq!(is_not_in_flag(read_flag, Flag::MATE_UNMAPPED), true);

[johanneskoester]

See the tab "files changed" at the top. There is still the old sam_flags.rs in this PR.

[rLannes]

Thank you for catching that, I was not aware I had to "git rm" old files to get rid of them. It should be good now.

[johanneskoester]

Something is wrong now.

[rLannes]

I am confused None of the linting errors have to do with the flag module. They're all about Clippy complaining about format!() in the whole code base.

[johanneskoester]

There are syntax errors in the code now. And further, the doc comments really should be put above the function definitions, with ///.