feat: fasta aligned action, and multi-sequence aamutation action #195
Conversation
Taepper
changed the title
There was a problem hiding this comment.
Can we test this action in a unit test (now that we can have sequences of arbitrary length)?
There was a problem hiding this comment.
I will maybe introduce those next week if it's okay. As we need a database for the query engine to run and don't have any such tests yet, we could maybe introduce a good framework for such tests, which I would do in a separate PR
There was a problem hiding this comment.
Note that the fastaAligned_multiple.json endToEndTest already tests this function with sequences of differing lengths
There was a problem hiding this comment.
I had the different edge and error cases in mind, e.g. the sequence name does not exist. And also testing that the resulting sequence is actually correct, since this is vital. The reconstruction of the sequence is relatively complex.
| for (auto position_id = local.begin(); position_id != local.end(); position_id++) { | ||
| const NucPosition& position = sequence_store.positions.at(position_id); | ||
| for (const NUCLEOTIDE_SYMBOL symbol : NUC_SYMBOLS) { | ||
| if (symbol != position.symbol_whose_bitmap_is_flipped && |
There was a problem hiding this comment.
Why is it possible to not consider positions with flipped bitmaps?
There was a problem hiding this comment.
This is indeed wrong if the flipped symbol is unequal to the reference sequence! Good catch!
| "AminoAcidMutations action can have the field sequenceName of type string or an array of " | ||
| "strings, but no other type" |
There was a problem hiding this comment.
| "AminoAcidMutations action can have the field sequenceName of type string or an array of " | |
| "strings, but no other type" | |
| "The field sequenceName of AminoAcidMutations action must have type string or an array, if present." |
| "AminoAcidMutations action can have the field sequenceName of type string or an array " | ||
| "of " | ||
| "strings, but no other type; while parsing array encountered the element " + |
There was a problem hiding this comment.
| "AminoAcidMutations action can have the field sequenceName of type string or an array " | |
| "of " | |
| "strings, but no other type; while parsing array encountered the element " + | |
| "AminoAcidMutations action sequenceNames must be of type string, but found " + |
|
How final is the implementation of the |
|
Not final at all, but I just wanted to have something working, while writing it out to a buffer will still take some time I suppose. Maybe introduce a answer size limit for this action until we implement the compressed and/or streaming download |
|
Ok, for some working solution, this is definitely fine, although maybe not the solution we should build upon in LAPIS. |
…ap is different from the reference sequence symbol
This PR introduces an action to retrieve fasta sequences. (Either for one or multiple sequences)
And also includes a fix for AAMutation action to work for multiple sequences as it is a similar logic