Skip to content

fix: build correct alt dna sequence for deletions#647

Open
katie-perry wants to merge 7 commits intostagingfrom
issue-447-fix-gnomad-vcf-to-protein
Open

fix: build correct alt dna sequence for deletions#647
katie-perry wants to merge 7 commits intostagingfrom
issue-447-fix-gnomad-vcf-to-protein

Conversation

@katie-perry
Copy link

@katie-perry katie-perry self-assigned this Feb 11, 2026
@katie-perry katie-perry requested a review from a team as a code owner February 11, 2026 21:41
@katie-perry katie-perry added bug Something isn't working priority:high High priority labels Feb 11, 2026
jarbesfeld
jarbesfeld previously approved these changes Feb 12, 2026
Copy link

@jarbesfeld jarbesfeld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@katie-perry Nice work! This looks good to me

aa_start_pos += aa_match
aa_alt = aa_alt[aa_match:] if trim_prefix else aa_alt[:-aa_match]
aa_ref = aa_ref[aa_match:] if trim_prefix else aa_ref[:-aa_ref]
aa_ref = aa_ref[aa_match:] if trim_prefix else aa_ref[:-aa_match]
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@korikuzma just wanted your eyes to double check on this - is this correct? It seemed like a bug so I changed it but it wasn't impacting any tests... maybe I should write some tests for this if there aren't already some

@jarbesfeld
Copy link

@katie-perry Could you add two additional checks in the test_deletion test for reading frame 1 negative strand and reading frame 2 positive strand? I also relocated one test that should've been in test_substitution. I used chat to help me generate that example, as well as the corresponding submission in ClinVar so I could validate what the protein change was.

@katie-perry
Copy link
Author

@katie-perry Could you add two additional checks in the test_deletion test for reading frame 1 negative strand and reading frame 2 positive strand? I also relocated one test that should've been in test_substitution. I used chat to help me generate that example, as well as the corresponding submission in ClinVar so I could validate what the protein change was.

great idea - I will work on this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working priority:high High priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants