-
Notifications
You must be signed in to change notification settings - Fork 5
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: Sec and Stop coincide in SEPHS2 #214
Conversation
WalkthroughThe changes in this pull request focus on enhancing the logic for handling alternative sequences related to genetic variants in the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
src/mapper/altseq.rs (2)
159-171
: Consider handling selenocysteine cases more robustlyThe heuristic introduced for handling cases where selenocysteine ('U') coincides with a stop codon may not cover all scenarios. Consider enhancing the logic to check against known
protein_accession
cases or making the handling configurable to ensure correctness across different sequences.
Line range hint
304-304
: Replacepanic!
with proper error handlingUsing
panic!
in production code can lead to unexpected crashes. It's advisable to return an error instead, allowing the calling function to handle it gracefully.Apply this diff to replace the
panic!
with an error return:- _ => panic!("Can only work with concrete ref/alt"), + _ => return Err(Error::InvalidVariant("Expected concrete ref/alt in NaEdit".to_string())),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/mapper/altseq.rs
(1 hunks)
🔇 Additional comments (1)
src/mapper/altseq.rs (1)
Line range hint 942-949
: Correctly truncating the alternative
sequence after stop codons
The added code appropriately truncates the alternative
sequence at the first occurrence of a stop codon ('*'
or 'X'
), which ensures that the resulting protein sequence is accurate and does not include residues beyond the stop codon.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #214 +/- ##
==========================================
- Coverage 92.20% 92.19% -0.01%
==========================================
Files 18 18
Lines 11440 11443 +3
==========================================
+ Hits 10548 10550 +2
- Misses 892 893 +1
|
supersedes #213
in HGNC:19686, the last amino acid is actually translated to selenocysteine, but is practically interpreted as stop as well.