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

Ambiguous overload in debug stream operator for tuple and aligned sequence #125

Closed
1 of 4 tasks
rrahn opened this issue Jun 11, 2020 · 3 comments · Fixed by seqan/seqan3#1896
Closed
1 of 4 tasks
Assignees
Labels
bug Something isn't working
Milestone

Comments

@rrahn
Copy link
Contributor

rrahn commented Jun 11, 2020

Description

The following code does not compile:

#include <sstream>
#include <vector>

#include <seqan3/alphabet/adaptation/char.hpp>
#include <seqan3/alignment/aligned_sequence/aligned_sequence_concept.hpp>
#include <seqan3/core/detail/debug_stream_tuple.hpp>

// When including aligned_sequence_concept and debug_stream_tuple the overload for the stream operator
// is ambiguous. The solution is to introduce a named concept for a debug streamble tuple type.
TEST(debug_stream_type, incompatible_debbug_stream_overload)
{
    using aligned_sequence_t = std::vector<seqan3::gapped<char>>;
    using alignment_t = std::pair<aligned_sequence_t, aligned_sequence_t>;

    std::ostringstream oss;
    seqan3::debug_stream_type stream{oss};
    stream << alignment_t{};
    EXPECT_EQ(oss.str(), "");
}

Tasks

  • Make the expression: (!std::ranges::input_range<tuple_t>) && (!alphabet<tuple_t>) && tuple_like<tuple_t> a named concept and reuse it in the overload for tuple and aligned sequence.

Definition of Done

  • Implementation and design approved
  • Unit tests pass
  • Test coverage = 100%
@rrahn
Copy link
Contributor Author

rrahn commented Jun 12, 2020

I currently don't like the solution to add the requirements of ranges and alphabet into core tuple concept.
Considering #60 I propose the following changes:

  1. move the debub stream overload in aligned sequence into its own header.
  2. make it dependent to the tuple overload as long as alignment concept subsumes tuple concept
  3. add the corresponding debug_streamable_tuple concept into the debug stream overload of tuple.

@marehr
Copy link
Member

marehr commented Jun 12, 2020

#63 addresses all of these issues. It is a shame that no-one read my problem list. I already have a design in mind to fix it "once and for all". When I asked to work on it, it was deemed as not important enough, so I postponed it. Now it seems relevant again and we only hotfix things and not the underlying problem of our current design.

@rrahn
Copy link
Contributor Author

rrahn commented Jun 12, 2020

That is a little strong opinion on this matter. I mean once this is fixed there will be other things and then there will be other things and so on and so forth. But we can only do as much as we can do. That is we need to add features the same way we need to get designs and dependencies in the backend straight. I mean honestly, do you have capacities to do this? So yes, it is important but sometimes other stuff is more important. Having all the standard conformance issues resolved is such a more important thing. Let's consider it for the next quarter though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants