[FEATURE] cigar.assign_string from char pointers. #2966
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Codecov Report
@@ Coverage Diff @@
## master #2966 +/- ##
=======================================
Coverage 98.20% 98.21%
=======================================
Files 274 274
Lines 12216 12216
=======================================
+ Hits 11997 11998 +1
+ Misses 219 218 -1
Continue to review full report at Codecov.
|
eseiler
reviewed
Apr 22, 2022
|
In C++23 we could do with a single overload that accepts cigar & assign_string(std::string_view const s) // not noexcept, should throw on problem
{
// implementation
}
template <std::ranges::contiguous_range rng_t>
requires (std::ranges::sized_range<rng_t> && std::same_as<char, std::ranges::range_value_t<rng_t>>)
cigar & assign_string(rng_t && s)
{
return assign_string(std::string_view{std::ranges::data(s), std::ranges::size(s)});
}I don't know why there should be an overload for |
h-2
reviewed
May 3, 2022
h-2
reviewed
May 18, 2022
h-2
reviewed
May 18, 2022
eseiler
reviewed
May 20, 2022
smehringer
commented
May 23, 2022
smehringer
commented
May 23, 2022
|
.
|
eseiler
force-pushed
the
cigar_string
branch
2 times, most recently
from
6f528db to
474d587
Compare
eseiler
reviewed
May 31, 2022
Comment on lines
162
to
170
| /*!\brief Assign from a contigous char array. | ||
| * \details | ||
| * \experimentalapi{Experimental since version 3.1.} | ||
| * | ||
| * In order to avoid unnecessary copies, you can initialise a seqan3::cigar from two char pointers that point to a | ||
| * contigous char array that stores the cigar string. | ||
| * | ||
| * \include test/snippet/alphabet/cigar/cigar_assign_string.cpp | ||
| * | ||
| * \attention If the cigar count cannot be correctly read, `0P` is added instead to the cigar string. | ||
| * Adding 0 padding is not impacting the alignment that the cigar string represents but | ||
| * keeps the number of cigar elements consistent with the input. | ||
| * | ||
| * \experimentalapi{Experimental since version 3.2.} |
There was a problem hiding this comment.
needs an update (still refers to two char const *)
seqan-actions
added
lint
seqan-actions
added
lint
eseiler
reviewed
Jun 3, 2022
seqan-actions
added
lint
eseiler
approved these changes
Jun 3, 2022
h-2
approved these changes
Jun 5, 2022
eseiler
reviewed
Jun 7, 2022
seqan-actions
added
lint
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
My use case is the following:
From an input file stream (a SAM file) I want to construct a
std::vector<seqan3::cigar>from a given cigar string e.g.3M. The current overloadseqan3::cigar::assign_string(seqan3::small_string s)would copy the string into asmall_stringbefore reading it and converting it to a respective letter ofseqan3::cigar. This is an unnecessary copy. We can avoid this by providing an overload withconst char *.