Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

implicit conversion of dna4 from rna4. #320

Closed
1 task
marehr opened this issue Apr 18, 2021 · 1 comment · Fixed by seqan/seqan3#2656
Closed
1 task

implicit conversion of dna4 from rna4. #320

marehr opened this issue Apr 18, 2021 · 1 comment · Fixed by seqan/seqan3#2656
Assignees

Comments

@marehr
Copy link
Member

marehr commented Apr 18, 2021

Tasks

  • implicit conversion between rna* and dna* should only accept our types
From 2a9acf73da20842103cbfd5d010f17b5f2ba5dfc Mon Sep 17 00:00:00 2001
From: marehr <marehr-github@marehr.dialup.fu-berlin.de>
Date: Thu, 11 Mar 2021 12:47:42 +0100
Subject: [PATCH 10/12] TODO: [DOC] implicit conversion of dna4 from rna4.

---
 include/seqan3/alphabet/nucleotide/dna4.hpp   | 28 +++++++++++++++++--
 include/seqan3/alphabet/nucleotide/rna4.hpp   |  2 +-
 .../dna4_implicit_conversion_from_rna4.cpp    | 11 ++++++++
 ..._implicit_conversion_from_rna4_inherit.cpp | 18 ++++++++++++
 ...4_implicit_conversion_from_rna4_vector.cpp | 19 +++++++++++++
 5 files changed, 74 insertions(+), 4 deletions(-)
 create mode 100644 test/snippet/alphabet/nucleotide/dna4_implicit_conversion_from_rna4.cpp
 create mode 100644 test/snippet/alphabet/nucleotide/dna4_implicit_conversion_from_rna4_inherit.cpp
 create mode 100644 test/snippet/alphabet/nucleotide/dna4_implicit_conversion_from_rna4_vector.cpp

diff --git a/include/seqan3/alphabet/nucleotide/dna4.hpp b/include/seqan3/alphabet/nucleotide/dna4.hpp
index 57140e826..de13a1bd1 100644
--- a/include/seqan3/alphabet/nucleotide/dna4.hpp
+++ b/include/seqan3/alphabet/nucleotide/dna4.hpp
@@ -76,9 +76,31 @@ public:
 
     using base_t::base_t;
 
-    //!\brief Allow implicit construction from dna/rna of the same size.
-    template <std::same_as<rna4> t>    // Accept incomplete type
-    constexpr dna4(t const & r) noexcept
+    /*!\brief Allow implicit construction from dna/rna of the same size.
+     * \details
+     *
+     * Normally, we don't allow implicit conversion of single argument constructors, but in this case we allow it,
+     * because seqan3::dna4 and seqan3::rna4 are interchangeable as they behave nearly the same (e.g. same ranks,
+     * same char to rank conversion).
+     *
+     * \include test/snippet/alphabet/nucleotide/dna4_implicit_conversion_from_rna4.cpp
+     *
+     * seqan3::sequence's (e.g. seqan3::dna4_vector) in general aren't implicitly convertible and must be
+     * explicitly copied to be converted:
+     *
+     * \include test/snippet/alphabet/nucleotide/dna4_implicit_conversion_from_rna4_vector.cpp
+     *
+     * You can avoid this copy by using std::ranges::view's:
+     *
+     * This only allows converting seqan3::rna4 to seqan3::dna4. Other alphabets that inherit from seqan3::rna4 will not
+     * be implicitly convertible to seqan3::dna4.
+     *
+     * \include test/snippet/alphabet/nucleotide/dna4_implicit_conversion_from_rna4_inherit.cpp
+     *
+     * \stableapi{Since version 3.1.}
+     */
+    template <std::same_as<rna4> t> // template parameter t to accept incomplete type
+    constexpr dna4(t const & r) noexcept // TODO check what breaks if explicit
     {
         assign_rank(r.to_rank());
     }
diff --git a/include/seqan3/alphabet/nucleotide/rna4.hpp b/include/seqan3/alphabet/nucleotide/rna4.hpp
index 4ecf8083f..ef91e183c 100644
--- a/include/seqan3/alphabet/nucleotide/rna4.hpp
+++ b/include/seqan3/alphabet/nucleotide/rna4.hpp
@@ -71,7 +71,7 @@ public:
     using base_t::base_t;
 
     //!\brief Allow implicit construction from dna/rna of the same size.
-    constexpr rna4(dna4 const & r) noexcept
+    constexpr rna4(dna4 const & r) noexcept // TODO: should behave the same as seqan3::dna4
 #if SEQAN3_WORKAROUND_GCC_90897
         requires true
 #endif
diff --git a/test/snippet/alphabet/nucleotide/dna4_implicit_conversion_from_rna4.cpp b/test/snippet/alphabet/nucleotide/dna4_implicit_conversion_from_rna4.cpp
new file mode 100644
index 000000000..f3c7e49e7
--- /dev/null
+++ b/test/snippet/alphabet/nucleotide/dna4_implicit_conversion_from_rna4.cpp
@@ -0,0 +1,11 @@
+#include <seqan3/alphabet/nucleotide/dna4.hpp>
+#include <seqan3/alphabet/nucleotide/rna4.hpp>
+
+int main()
+{
+    using seqan3::operator""_rna4;
+
+    seqan3::dna4 letter1 = 'C'_rna4; // implicitly converted
+    seqan3::dna4 letter2{};
+    letter2 = 'C'_rna4; // implicitly converted
+}
diff --git a/test/snippet/alphabet/nucleotide/dna4_implicit_conversion_from_rna4_inherit.cpp b/test/snippet/alphabet/nucleotide/dna4_implicit_conversion_from_rna4_inherit.cpp
new file mode 100644
index 000000000..8e1a3ef5d
--- /dev/null
+++ b/test/snippet/alphabet/nucleotide/dna4_implicit_conversion_from_rna4_inherit.cpp
@@ -0,0 +1,18 @@
+#include <seqan3/alphabet/nucleotide/dna4.hpp>
+#include <seqan3/alphabet/nucleotide/rna4.hpp>
+
+struct my_dna4 : public seqan3::dna4
+{
+    // using seqan3::dna4::dna4; // uncomment to import implicit conversion shown by letter1
+};
+
+struct my_rna4 : public seqan3::rna4
+{};
+
+int main()
+{
+    using seqan3::operator""_rna4;
+
+    // my_dna4 letter1 = 'C'_rna4; // NO automatic implicit conversion!
+    // seqan3::dna4 letter2 = my_rna4{}; // seqan3::dna4 only allows implicit conversion from seqan3::rna4!
+}
diff --git a/test/snippet/alphabet/nucleotide/dna4_implicit_conversion_from_rna4_vector.cpp b/test/snippet/alphabet/nucleotide/dna4_implicit_conversion_from_rna4_vector.cpp
new file mode 100644
index 000000000..e052fa5ee
--- /dev/null
+++ b/test/snippet/alphabet/nucleotide/dna4_implicit_conversion_from_rna4_vector.cpp
@@ -0,0 +1,19 @@
+#include <vector>
+
+#include <seqan3/alphabet/nucleotide/dna4.hpp>
+#include <seqan3/alphabet/nucleotide/rna4.hpp>
+
+int main()
+{
+    using seqan3::operator""_rna4;
+
+    seqan3::dna4_vector vector{'A'_rna4, 'C'_rna4, 'G'_rna4}; // (element-wise) implicit conversion
+
+    // but this won't work:
+    // seqan3::dna4_vector dna4_vector{"ACGT"_rna4};
+
+    // as a workaround you can use:
+    // side note: this would also work without the implicit conversion.
+    seqan3::rna4_vector rna4_vector = "ACGT"_rna4;
+    seqan3::dna4_vector dna4_vector{rna4_vector.begin(), rna4_vector.end()};
+}
-- 
2.31.1
@marehr
Copy link
Member Author

marehr commented May 18, 2021

fixed by seqan/seqan3#2656

@marehr marehr closed this as completed May 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant