WIP on map_io #14
WIP on map_io #14
Conversation
| * is not in a correct state (e.g. required fields are not given), but throwing might occur downstream of the actual | ||
| * error. | ||
| */ | ||
| void header::read(std::string_view header_string) |
There was a problem hiding this comment.
This could be made private if the header is constructed from a full-header-string.
| struct sam_tag_type | ||
| { | ||
| //!\brief The type for all unknown tags with no extra overload defaults to a std::variant. | ||
| using type = detail::sam_tag_variant; |
There was a problem hiding this comment.
If we expose this, should it be public? The variants in var_io are public... In general, the interface of the sam_tag_dictionary is very different from how this is done in var_io at the moment.
I would recommend merging the dictionary as-is, and that we later re-evaluate together whether a common design makes sense.
There was a problem hiding this comment.
ok, I just copied over the seqan3 version because I didn't have time to go into every detail of the VCF implementation. Let's again keep track of this.
| BIO_RECORD_MEMBER(tlen) | ||
| BIO_RECORD_MEMBER(optionals) | ||
| BIO_RECORD_MEMBER(tags) |
There was a problem hiding this comment.
| BIO_RECORD_MEMBER(tags) | |
| BIO_RECORD_MEMBER(sam_tags) |
? Dunno. Maybe tags is OK, too.
There was a problem hiding this comment.
hm.. none of the other names is prefixed with its format. E.g. tlen also does not really tell you anything.
On the other hand, tags is I think also made up from me :D
Here is the respective paragraph form the SAM specs:
The alignment section: optional fields
All optional fields follow the TAG:TYPE:VALUE format where TAG is a two-character string that matches
/[A-Za-z][A-Za-z0-9]/. Within each alignment line, no TAG may appear more than once and the order
in which the optional fields appear is not significant. A TAG containing lowercase letters is reserved for end
users. In an optional field, TYPE is a single case-sensitive letter which defines the format of VALUE:
because it is called TAG:VALUE I guess I called it tags. I think it's more memorable than optional_fields? But I have no strong feelings on this. I am just against prefixing the format.
There was a problem hiding this comment.
I don't like tag because we have already other things called "tag" and it has a very specific meaning in C++ (it's always difficult when a term has one meaning in C++ and another in Bioinformatics).
I also decided to stay close to the format-terminology, so I would propose to just call the field "optionals".
| #pragma GCC diagnostic push | ||
| #pragma GCC diagnostic ignored "-Wpedantic" | ||
| template <typename char_t, char_t... s> | ||
| constexpr uint16_t operator""_tag() |
There was a problem hiding this comment.
I would prefer if we can replace this with a bio::vtag, but this doesn't have to happen now. I will have a think on it.
There was a problem hiding this comment.
Alright, are you keeping track of this?
There was a problem hiding this comment.
I have now started adding Github discussion items for these things. Feel free to add information there.
Co-authored-by: Hannes Hauswedell <[email protected]>
No description provided.