[FEATURE] seqan3::sam_record

[marehr]

Part of seqan/product_backlog#289

This implements

• sam_record
• adds member to sam_record, see names IO: make records explicit types + member functions product_backlog#289
• and does some infrastructure

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/seqan/seqan3/H2c7c6cNSGeR9Nv7A1SCR8D5UVFs
✅ Preview: https://seqan3-git-fork-marehr-ioexplicitrecordtypenr3-seqan.vercel.app

[codecov]

Codecov Report

Merging #2389 (d59e290) into master (6f39b37) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #2389   +/-   ##
=======================================
  Coverage   98.20%   98.21%           
=======================================
  Files         266      267    +1     
  Lines       10831    10864   +33     
=======================================
+ Hits        10637    10670   +33     
  Misses        194      194           

Impacted Files	Coverage Δ
include/seqan3/io/alignment_file/input.hpp	100.00% <ø> (ø)
include/seqan3/io/alignment_file/record.hpp	100.00% <100.00%> (ø)
include/seqan3/io/record.hpp	100.00% <0.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6f39b37...d59e290. Read the comment docs.

[eaasna]

Thanks a lot for the effort!

Just a small correction and a question

[eaasna]

Could you please explain why there are 4 different assignments for each field? The slight differences in syntax are not clear to me

[marehr]

Yes, we want to mimic the way std::get behaves, and it will return

• id_type && for std::get<id>(sam_record &&)
• id_type & for std::get<id>(sam_record &)
• id_type const && for std::get<id>(sam_record const &&)
• id_type const & for std::get<id>(sam_record const &)

We unfortunately need 4 overloads for this depending on whether the input has the type modifier &&, &, const &&, const &. The return type, here decltype(auto), has the corresponding type modifier hidden within it.

This is a special case, and this is not the norm, so you should not write 4 accessors functions if you write code yourself. In most cases it is fine just to copy the data without having a ref/const-qualified function.

[eaasna]

Thanks for the explanation!

[eseiler]

Just documentation stuff, I'm not sure about all the phrasings either

[marehr]

@eseiler I added a couple of suggestions, can you go over them and edit and then apply them? You are completely right that seqan3::field::ref_seq aka seqan3::sam_record::reference_sequence isn't populated at all, and I "deleted" it for now.

[eseiler]

@eseiler I added a couple of suggestions, can you go over them and edit and then apply them? You are completely right that seqan3::field::ref_seq aka seqan3::sam_record::reference_sequence isn't populated at all, and I "deleted" it for now.

I thought you can construct the alignment_file_input with additional reference information, and then it works?

[marehr]

@eseiler I added a couple of suggestions, can you go over them and edit and then apply them? You are completely right that seqan3::field::ref_seq aka seqan3::sam_record::reference_sequence isn't populated at all, and I "deleted" it for now.

I thought you can construct the alignment_file_input with additional reference information, and then it works?

You are right, I can add this if you want. I thought of this Pr as a reintroduction of existing, working functionality, but this would be more like a new "feature". I'd want to do it in a second PR, because I need to add new test cases for it.

[eseiler]

@eseiler I added a couple of suggestions, can you go over them and edit and then apply them? You are completely right that seqan3::field::ref_seq aka seqan3::sam_record::reference_sequence isn't populated at all, and I "deleted" it for now.

I thought you can construct the alignment_file_input with additional reference information, and then it works?

You are right, I can add this if you want. I thought of this Pr as a reintroduction of existing, working functionality, but this would be more like a new "feature". I'd want to do it in a second PR, because I need to add new test cases for it.

Ah, so it works for the alignment_file_record, but not for the sam_record? This is ok for me.

Do you want to rebase?

[marehr]

@eseiler I added a couple of suggestions, can you go over them and edit and then apply them? You are completely right that seqan3::field::ref_seq aka seqan3::sam_record::reference_sequence isn't populated at all, and I "deleted" it for now.

I thought you can construct the alignment_file_input with additional reference information, and then it works?

You are right, I can add this if you want. I thought of this Pr as a reintroduction of existing, working functionality, but this would be more like a new "feature". I'd want to do it in a second PR, because I need to add new test cases for it.

Ah, so it works for the alignment_file_record, but not for the sam_record? This is ok for me.

Do you want to rebase?

I added a backlog issue seqan/product_backlog#292