Review of #164 #165
Review of #164 #165
Conversation
|
|
||
| /// Result of analyzing blocks for reconsensus operation | ||
| #[derive(Clone, Debug, Serialize, Deserialize)] | ||
| struct BlockAnalysis { |
There was a problem hiding this comment.
Added Edit to BlockAnalysis to avoid re-computing and taking decisions on separate objects
| } | ||
|
|
||
| /// Applies full reconsensus including indels and realignment | ||
| fn apply_full_reconsensus( |
There was a problem hiding this comment.
Moved this method to the Block class edit_consensus_and_realign.
Given a set of edits that method changes the block consensus and re-aligns all the sequences
|
|
||
| /// Change a nucleotide in the consensus sequence at a specific position by applying a substitution. | ||
| /// Updates the alignment substitutions accordingly, without changing the underlying sequences. | ||
| pub fn change_consensus_nucleotide_at_pos(&mut self, s: &Sub) -> Result<(), Report> { |
There was a problem hiding this comment.
Given a substitution, it applies it to the block consensus sequence and updates all alignment edits for consistency.
|
|
||
| /// Applies a set of edits to the block's consensus sequence and re-aligns the sequences | ||
| /// to the new consensus. Returns a new `PangraphBlock` object with the same BlockId. | ||
| pub fn edit_consensus_and_realign(self, edits: &Edit, args: &PangraphBuildArgs) -> Result<Self, Report> { |
There was a problem hiding this comment.
Given a set of edits (also indels) it applies them to the block consensus, and then re-aligns all block sequences to the new consensus.
- I made it return a new block instead of modifying the one existing in-place, since it has to store the new sequences anyways
- I did not parallelize the re-alignment since this should be a relatively rare operation and I don't expect it to matter much...
| } | ||
|
|
||
| /// Updates the consensus sequence of the block and re-aligns the sequences to the new consensus. | ||
| fn update_block_consensus( |
There was a problem hiding this comment.
moved this method to the Block class edit_consensus_and_realign
| // Then apply indels and realign if present | ||
| if majority_edits.has_indels() { | ||
| let consensus = block.consensus(); | ||
| let new_consensus = majority_edits.apply(consensus)?; |
There was a problem hiding this comment.
this was applying substitutions twice and is now removed.
ivan-aksamentov
merged commit dcf6c32
into
refactor/reconsensus-updated
2 participants
Review of #164
BlockAnalysisto avoid recomputing edits (can be removed if not needed, the save should be minimal, it's more a matter of computing the object just once to avoid confusion...)apply_full_reconsensus, avoiding dealing with substitutions twice.