remove ref_seq, evalue, bit_score from sam file output (#2658)

* [MISC] deprecate seqan3::sam_file::output fields (seqan3::field::{ref_seq, evalue, bit_score}) sam_file_output does not need the fields ref_seq, evalue, bit_score. This commit remove those fields from the default-fields and ensures that custom fields don't contain those fields. This commit also splits ::push_back and ::emplace_back into four versions to deprecate the usage of the old default-fields case. * [MISC] deprecate seqan3::sam_file_output::{emplace, push}_back * [MISC] deprecate seqan3::sam_file::input fields (seqan3::field::{ref_seq, evalue, bit_score}) * [MISC] fix gcc-7 issues with SEQAN3_DEPRECATED_310 * [FIX] remove seqan3::field::ref_seq from benchmark * [DOC] add CHANGELOG * Apply suggestions from code review * Apply suggestions from code review * Update CHANGELOG.md Co-authored-by: Enrico Seiler <[email protected]>
seqan · May 19, 2021 · fb57135 · fb57135
1 parent ca7d9cc
commit fb57135
Show file tree

Hide file tree

Showing 10 changed files with 299 additions and 27 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -267,6 +267,9 @@ Header Changes:
   * `seqan3::alignment_file_output` is replaced by `seqan3::sam_file_output`.
   * `seqan3::alignment_file_output_format` is replaced by `seqan3::sam_file_output_format`.
   * `seqan3::alignment_file_output_options` is replaced by `seqan3::sam_file_output_options`.
+* `seqan3::sam_file_input` and `seqan3::sam_file_output` do not accept `seqan3::field::ref_seq`,
+  `seqan3::field::evalue` and `seqan3::field::bit_score` anymore.
+  ([\#2658](https://github.com/seqan/seqan3/pull/2658)).
 * The `seqan3::get` accessor for I/O records, e.g. `seqan3::get<seqan3::field::id>(record)`, is deprecated, please use
   the corresponding member accessor ([\#2420](https://github.com/seqan/seqan3/pull/2420)):
   * If you used files as views with `seqan3::views::get<seqan3::field::id>` to project a single field, e.g.

diff --git a/include/seqan3/io/alignment_file/output.hpp b/include/seqan3/io/alignment_file/output.hpp
@@ -25,7 +25,6 @@ template <detail::fields_specialisation selected_field_ids_ =
               fields<field::seq,
                      field::id,
                      field::offset,
-                     field::ref_seq,
                      field::ref_id,
                      field::ref_offset,
                      field::alignment,
@@ -35,8 +34,6 @@ template <detail::fields_specialisation selected_field_ids_ =
                      field::flag,
                      field::mate,
                      field::tags,
-                     field::evalue,
-                     field::bit_score,
                      field::header_ptr>,
           detail::type_list_of_sam_file_output_formats valid_formats_ = type_list<format_sam, format_bam>,
           typename ref_ids_type = ref_info_not_given>

diff --git a/include/seqan3/io/record.hpp b/include/seqan3/io/record.hpp
@@ -173,6 +173,9 @@ struct fields
     //!\brief Special value that indicates that index_of() failed.
     static constexpr size_t npos = std::numeric_limits<size_t>::max();
 
+    //!\brief The size of fields.
+    static constexpr size_t size = sizeof...(fs);
+
     //!\brief Retrieve the position of field in the parameter pack.
     static constexpr size_t index_of(field f)
     {

diff --git a/include/seqan3/io/sam_file/input.hpp b/include/seqan3/io/sam_file/input.hpp
@@ -512,13 +512,16 @@ class sam_file_input
 #endif // SEQAN3_DEPRECATED_310
                              field::header_ptr>;
 
-#ifndef SEQAN3_DEPRECATED_310
-static_assert(!field_ids::contains(field::ref_seq),
-              "You selected the deprecated seqan3::field::ref_seq. It will not be available in the record.");
-static_assert(!field_ids::contains(field::evalue),
-              "You selected the deprecated seqan3::field::evalue. It will not be available in the record.");
-static_assert(!field_ids::contains(field::bit_score),
-              "You selected the deprecated seqan3::field::bit_score. It will not be available in the record.");
+#ifdef SEQAN3_DEPRECATED_310
+    //!brief Does selected_field_ids contain all fields like in the default case?
+    static constexpr bool is_default_selected_field_ids = selected_field_ids::size == field_ids::size;
+
+    static_assert(is_default_selected_field_ids || !selected_field_ids::contains(field::ref_seq),
+                  "You selected the deprecated seqan3::field::ref_seq. It will not be available in the record.");
+    static_assert(is_default_selected_field_ids || !selected_field_ids::contains(field::evalue),
+                  "You selected the deprecated seqan3::field::evalue. It will not be available in the record.");
+    static_assert(is_default_selected_field_ids || !selected_field_ids::contains(field::bit_score),
+                  "You selected the deprecated seqan3::field::bit_score. It will not be available in the record.");
 #endif // SEQAN3_DEPRECATED_310
 
     static_assert([] () constexpr

diff --git a/include/seqan3/io/sam_file/output.hpp b/include/seqan3/io/sam_file/output.hpp
@@ -40,6 +40,18 @@
 namespace seqan3
 {
 
+#ifdef SEQAN3_DEPRECATED_310
+//!\cond
+template <typename tuple_t>
+static constexpr bool fourth_tuple_element_is_sequence = false;
+
+template <typename tuple_t>
+    requires (seqan3::tuple_like<tuple_t> && std::tuple_size_v<std::remove_cvref_t<tuple_t>> > 3)
+static constexpr bool fourth_tuple_element_is_sequence<tuple_t>
+    = seqan3::sequence<std::remove_cvref_t<std::tuple_element_t<3, tuple_t>>>;
+//!\endcond
+#endif // SEQAN3_DEPRECATED_310
+
 // ----------------------------------------------------------------------------
 // sam_file_output
 // ----------------------------------------------------------------------------
@@ -145,7 +157,6 @@ template <detail::fields_specialisation selected_field_ids_ =
               fields<field::seq,
                      field::id,
                      field::offset,
-                     field::ref_seq,
                      field::ref_id,
                      field::ref_offset,
                      field::alignment,
@@ -155,8 +166,6 @@ template <detail::fields_specialisation selected_field_ids_ =
                      field::flag,
                      field::mate,
                      field::tags,
-                     field::evalue,
-                     field::bit_score,
                      field::header_ptr>,
           detail::type_list_of_sam_file_output_formats valid_formats_ = type_list<format_sam, format_bam>,
           typename ref_ids_type = ref_info_not_given>
@@ -176,17 +185,15 @@ class sam_file_output
     //!\deprecated This will be removed in 3.1.0.
     using ref_ids_type_t        = ref_ids_type;
 //!\publicsection
-#endif
+#endif // SEQAN3_DEPRECATED_310
     //!\brief Character type of the stream(s).
     using stream_char_type      = char;
     //!\}
 
     //!\brief The subset of seqan3::field IDs that are valid for this file.
-    using field_ids             = fields<field::header_ptr,
-                                         field::seq,
+    using field_ids             = fields<field::seq,
                                          field::id,
                                          field::offset,
-                                         field::ref_seq,
                                          field::ref_id,
                                          field::ref_offset,
                                          field::alignment,
@@ -196,8 +203,19 @@ class sam_file_output
                                          field::qual,
                                          field::mate,
                                          field::tags,
-                                         field::evalue,
-                                         field::bit_score>;
+                                         field::header_ptr>;
+
+#ifdef SEQAN3_DEPRECATED_310
+    //!brief Does selected_field_ids contain all fields like in the default case?
+    static constexpr bool is_default_selected_field_ids = selected_field_ids::size == field_ids::size;
+
+    static_assert(is_default_selected_field_ids || !selected_field_ids::contains(field::ref_seq),
+                  "You selected the deprecated seqan3::field::ref_seq. It will not be available in the record.");
+    static_assert(is_default_selected_field_ids || !selected_field_ids::contains(field::evalue),
+                  "You selected the deprecated seqan3::field::evalue. It will not be available in the record.");
+    static_assert(is_default_selected_field_ids || !selected_field_ids::contains(field::bit_score),
+                  "You selected the deprecated seqan3::field::bit_score. It will not be available in the record.");
+#endif // SEQAN3_DEPRECATED_310
 
     static_assert([] () constexpr
                   {
@@ -534,11 +552,116 @@ class sam_file_output
      *
      * \include test/snippet/io/sam_file/push_back_tuple.cpp
      */
+#ifdef SEQAN3_DEPRECATED_310
+    template <typename tuple_t>
+    void push_back(tuple_t && t)
+    //!\cond
+        // new syntax enforces via static_assert that field::ref_seq, field::evalue, and field::bit_score isn't set
+        requires tuple_like<tuple_t> && (!detail::record_like<tuple_t>) && (!is_default_selected_field_ids)
+    //!\endcond
+    {
+        push_back_tuple(std::forward<tuple_t>(t));
+    }
+
+    // The new syntax enforces via static_assert (see above) that field::ref_seq, field::evalue, and field::bit_score
+    // isn't set. That makes sure that someone who explicitly requested for these fields will get a deprecation warning.
+    // That leaves the case where the user did not provide any fields and just used the default ones. 
+    // This causes a problem, because it is complicated to decide whether the new syntax or the old syntax is used.
+    // position | old fields         | new fields
+    // -------- | ------------------ | -------------
+    //  0       | field::seq         | field::seq
+    //  1       | field::id          | field::id
+    //  2       | field::offset      | field::offset
+    //  3       | field::ref_seq     | field::ref_id
+    //  4       | field::ref_id      | field::ref_offset
+    //  5       | field::ref_offset  | field::alignment
+    //  6       | field::alignment   | field::cigar
+    //  7       | field::cigar       | field::mapq
+    //  8       | field::mapq        | field::flag
+    //  9       | field::flag        | field::qual
+    // 10       | field::qual        | field::mate
+    // 11       | field::mate        | field::tags
+    // 12       | field::tags        | field::header_ptr
+    // 13       | field::evalue      | -
+    // 14       | field::bit_score   | -
+    // 15       | field::header_ptr  | -
+    //
+    // That means we have the following three cases:
+    // * sizeof...(arg_types) + 1 <= 3 are identical.
+    // * sizeof...(arg_types) + 1 > 3
+    //   * 4. arg is not a seqan3::sequence => that field is a field::ref_id (new syntax)
+    //   * 4. arg is a seqan3::sequence => that field is a field::ref_seq (old syntax)
+
+    //!\cond
+    // selected_field_ids::size can only be <= 13 or == 16 (static asserts above ensure that)
+    // sizeof...(arg_types) + 1 <= 3 are identical. (old and new syntax)
+    template <typename tuple_t>
+    void push_back(tuple_t && t)
+        requires tuple_like<tuple_t> && (!detail::record_like<tuple_t>) && (is_default_selected_field_ids)
+                 && (std::tuple_size_v<std::remove_cvref_t<tuple_t>> <= 3)
+    {
+        push_back_tuple(std::forward<tuple_t>(t));
+    }
+    //!\endcond
+
+    //!\cond
+    // * sizeof...(arg_types) + 1 > 3
+    //   * 4. arg is not a seqan3::sequence => that field is a field::ref_id (new syntax)
+    template <typename tuple_t>
+    void push_back(tuple_t && t)
+        requires tuple_like<tuple_t> && (!detail::record_like<tuple_t>) && (is_default_selected_field_ids)
+                 && (std::tuple_size_v<std::remove_cvref_t<tuple_t>> > 3)
+                 && (!fourth_tuple_element_is_sequence<std::remove_cvref_t<tuple_t>>)
+    {
+        push_back_tuple(std::forward<tuple_t>(t));
+    }
+    //!\endcond
+
+    //!\cond
+    // * sizeof...(arg_types) + 1 > 3
+    //   * 4. arg is a seqan3::sequence => that field is a field::ref_seq (old syntax)
+    template <typename tuple_t>
+    SEQAN3_DEPRECATED_310 void push_back(tuple_t && t)
+        requires tuple_like<tuple_t> && (!detail::record_like<tuple_t>) && (is_default_selected_field_ids)
+                 && (std::tuple_size_v<std::remove_cvref_t<tuple_t>> > 3)
+                 && (fourth_tuple_element_is_sequence<std::remove_cvref_t<tuple_t>>)
+    {
+        using default_align_t = std::pair<std::span<gapped<char>>, std::span<gapped<char>>>;
+        using default_mate_t  = std::tuple<std::string_view, std::optional<int32_t>, int32_t>;
+
+        // pad it to 16 elements (we know the order of the default field ids)
+        push_back(std::tie(
+            detail::get_or<0>(t, std::string_view{}), // field::seq
+            detail::get_or<1>(t, std::string_view{}), // field::id
+            detail::get_or<2>(t, 0u), // field::offset
+            // detail::get_or<3>(t, std::string_view{}), // field::ref_seq not used
+            detail::get_or<4>(t, std::ignore), // field::ref_id
+            detail::get_or<5>(t, std::optional<int32_t>{}), // field::ref_offset
+            detail::get_or<6>(t, default_align_t{}), // field::alignment
+            detail::get_or<7>(t, std::vector<cigar>{}), // field::cigar
+            detail::get_or<8>(t, 0u), // field::mapq
+            detail::get_or<9>(t, sam_flag::none), // field::flag
+            detail::get_or<10>(t, std::string_view{}), // field::qual
+            detail::get_or<11>(t, default_mate_t{}), // field::mate
+            detail::get_or<12>(t, sam_tag_dictionary{}),  // field::tags
+            // detail::get_or<13>(t, 0u), // field::evalue not used
+            // detail::get_or<14>(t, 0u), // field::bit_score not used
+            detail::get_or<15>(t, nullptr) // field::header_ptr
+        ));
+    }
+    //!\endcond
+
+    //!\brief Push back via tuple interface
+    //!\private
+    template <typename tuple_t>
+    void push_back_tuple(tuple_t && t)
+#else // ^^^ before SeqAn 3.1.0 / after SeqAn 3.1.0 vvv
     template <typename tuple_t>
     void push_back(tuple_t && t)
     //!\cond
         requires tuple_like<tuple_t> && (!detail::record_like<tuple_t>)
     //!\endcond
+#endif // SEQAN3_DEPRECATED_310
     {
         using default_align_t = std::pair<std::span<gapped<char>>, std::span<gapped<char>>>;
         using default_mate_t  = std::tuple<std::string_view, std::optional<int32_t>, int32_t>;
@@ -586,10 +709,62 @@ class sam_file_output
      * \include test/snippet/io/sam_file/emplace_back.cpp
      */
     template <typename arg_t, typename ...arg_types>
+    //!\cond
+        requires (sizeof...(arg_types) + 1 <= selected_field_ids::size)
+    //!\endcond
+    void emplace_back(arg_t && arg, arg_types && ... args)
+#ifdef SEQAN3_DEPRECATED_310
+        // selected_field_ids::size can only be <= 13 or == 16 (static asserts above ensure that)
+        requires (!is_default_selected_field_ids)
+#endif // SEQAN3_DEPRECATED_310
+    {
+        push_back(std::tie(arg, args...));
+    }
+
+#ifdef SEQAN3_DEPRECATED_310
+    //!\cond
+    // sizeof...(arg_types) + 1 <= 3 are identical. (old and new syntax)
+    template <typename arg_t, typename ...arg_types>
+        requires (sizeof...(arg_types) + 1 <= 3)
     void emplace_back(arg_t && arg, arg_types && ... args)
+        requires is_default_selected_field_ids
     {
         push_back(std::tie(arg, args...));
     }
+    //!\endcond
+
+    //!\cond
+    // * sizeof...(arg_types) + 1 > 3
+    //   * 4. arg is not a seqan3::sequence => that field is a field::ref_id (new syntax)
+    template <typename seq_t, typename id_t, typename offset_t, typename ref_id_t, typename ...arg_types>
+        requires (!seqan3::sequence<std::remove_cvref_t<ref_id_t>>)
+    void emplace_back(seq_t && seq, id_t && id, offset_t && offset, ref_id_t && ref_id, arg_types && ... args)
+        requires is_default_selected_field_ids
+    {
+        push_back(std::tie(seq, id, offset, ref_id, args...));
+    }
+    //!\endcond
+
+    //!\cond
+    // * sizeof...(arg_types) + 1 > 3
+    //   * 4. arg is a seqan3::sequence => that field is a field::ref_seq (old syntax)
+    template <typename seq_t, typename id_t, typename offset_t, typename ref_seq_t, typename ...arg_types>
+        requires seqan3::sequence<std::remove_cvref_t<ref_seq_t>>
+    void emplace_back SEQAN3_DEPRECATED_310(seq_t && seq,
+                                            id_t && id,
+                                            offset_t && offset,
+                                            [[maybe_unused]] ref_seq_t && ref_seq,
+                                            arg_types && ... args)
+        requires is_default_selected_field_ids
+    {
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
+        // use deprecated call
+        push_back(std::tie(seq, id, offset, ref_seq, args...));
+#pragma GCC diagnostic pop
+    }
+    //!\endcond
+#endif // SEQAN3_DEPRECATED_310
 
     /*!\brief            Write a range of records (or tuples) to the file.
      * \tparam rng_t     Type of the range, must satisfy std::ranges::output_range and have a reference type that

diff --git a/test/api_stability/3.0.2/0038-MISC-deprecate-seqan3-sam_file-output-fields-seqan3-.patch b/test/api_stability/3.0.2/0038-MISC-deprecate-seqan3-sam_file-output-fields-seqan3-.patch
@@ -0,0 +1,67 @@
+From 59fcd58ab4bd75891cf9ba99b04fdd716d63993a Mon Sep 17 00:00:00 2001
+From: marehr <[email protected]>
+Date: Sun, 16 May 2021 16:01:05 +0200
+Subject: [PATCH 38/38] [MISC] deprecate seqan3::sam_file::output fields
+ (seqan3::field::{ref_seq, evalue, bit_score})
+
+cherry-picked
+
+----
+
+sam_file_output does not need the fields ref_seq, evalue, bit_score.
+
+This commit remove those fields from the default-fields and ensures
+that custom fields don't contain those fields.
+
+This commit also splits ::push_back and ::emplace_back into four
+versions to deprecate the usage of the old default-fields case.
+---
+ ...ile_input_construction_without_automatic_type_deduction.cpp | 3 ---
+ test/unit/io/alignment_file/alignment_file_output_test.cpp     | 3 ---
+ 2 files changed, 6 deletions(-)
+
+diff --git a/test/snippet/io/alignment_file/alignment_file_input_construction_without_automatic_type_deduction.cpp b/test/snippet/io/alignment_file/alignment_file_input_construction_without_automatic_type_deduction.cpp
+index 137ff05d5..80ff04c64 100644
+--- a/test/snippet/io/alignment_file/alignment_file_input_construction_without_automatic_type_deduction.cpp
++++ b/test/snippet/io/alignment_file/alignment_file_input_construction_without_automatic_type_deduction.cpp
+@@ -12,7 +12,6 @@ int main()
+     using default_fields = seqan3::fields<seqan3::field::seq,
+                                           seqan3::field::id,
+                                           seqan3::field::offset,
+-                                          seqan3::field::ref_seq,
+                                           seqan3::field::ref_id,
+                                           seqan3::field::ref_offset,
+                                           seqan3::field::alignment,
+@@ -21,8 +20,6 @@ int main()
+                                           seqan3::field::flag,
+                                           seqan3::field::mate,
+                                           seqan3::field::tags,
+-                                          seqan3::field::evalue,
+-                                          seqan3::field::bit_score,
+                                           seqan3::field::header_ptr>;
+
+                                                                 // The expected format:
+diff --git a/test/unit/io/alignment_file/alignment_file_output_test.cpp b/test/unit/io/alignment_file/alignment_file_output_test.cpp
+index 489c78b7f..7ee1f79f5 100644
+--- a/test/unit/io/alignment_file/alignment_file_output_test.cpp
++++ b/test/unit/io/alignment_file/alignment_file_output_test.cpp
+@@ -118,7 +118,6 @@ TEST(general, default_template_args_and_deduction_guides)
+     using comp1 = seqan3::fields<seqan3::field::seq,
+                                  seqan3::field::id,
+                                  seqan3::field::offset,
+-                                 seqan3::field::ref_seq,
+                                  seqan3::field::ref_id,
+                                  seqan3::field::ref_offset,
+                                  seqan3::field::alignment,
+@@ -128,8 +127,6 @@ TEST(general, default_template_args_and_deduction_guides)
+                                  seqan3::field::flag,
+                                  seqan3::field::mate,
+                                  seqan3::field::tags,
+-                                 seqan3::field::evalue,
+-                                 seqan3::field::bit_score,
+                                  seqan3::field::header_ptr>;
+     using comp2 = seqan3::type_list<seqan3::format_sam, seqan3::format_bam>;
+     using comp3 = char;
+-- 
+2.31.1
+