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

[FIX] Relax sequence_file_input_traits #3128

Merged
merged 1 commit into from
Apr 25, 2023
Merged

Conversation

eseiler
Copy link
Member

@eseiler eseiler commented Jan 27, 2023

explicitly_convertible_to<typename t::sequence_legal_alphabet, typename t::sequence_alphabet> is meant for allowing, e.g.:

using sequence_alphabet = seqan3::dna4;
using sequence_legal_alphabet = seqan3::dna15; // or even seqan3::rna15

but not stuff like

using sequence_alphabet = seqan3::dna4;
using sequence_legal_alphabet = seqan3::aa27;

However, this constraint is also checked for using sequence_alphabet = char. Our alphabets are not (explicitly/implicitly) convertible to char.
The rationale of the alphabets being compatible doesn't apply for sequence_alphabet = char, so we should just allow it.

using sequence_alphabet = char;
using sequence_legal_alphabet = seqan3::dna4;

means: Only allow characters of seqan3::dna4, but store them as char.

@vercel
Copy link

vercel bot commented Jan 27, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
seqan3 ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 25, 2023 10:30am

@seqan-actions seqan-actions added lint [INTERNAL] signal for linting and removed lint [INTERNAL] signal for linting labels Jan 27, 2023
@codecov
Copy link

codecov bot commented Jan 27, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (aa502ef) 98.18% compared to head (11dc477) 98.18%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3128   +/-   ##
=======================================
  Coverage   98.18%   98.18%           
=======================================
  Files         276      276           
  Lines       12294    12294           
=======================================
  Hits        12071    12071           
  Misses        223      223           
Impacted Files Coverage Δ
include/seqan3/io/sequence_file/input.hpp 100.00% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@eseiler eseiler force-pushed the master branch 4 times, most recently from caa4f4c to a719fb0 Compare February 13, 2023 15:34
@seqan-actions seqan-actions added lint [INTERNAL] signal for linting and removed lint [INTERNAL] signal for linting labels Apr 25, 2023
@SGSSGene SGSSGene self-requested a review April 25, 2023 11:07
@eseiler eseiler removed the request for review from smehringer April 25, 2023 11:25
@eseiler eseiler merged commit eb907b8 into seqan:master Apr 25, 2023
@eseiler eseiler deleted the fix/relax branch April 25, 2023 11:25
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 this pull request may close these issues.

3 participants