Skip to content

feat: Address deletion and substitution bugs in gnomad_vcf_to_protein_variation.py#646

Closed
jarbesfeld wants to merge 4 commits intostagingfrom
gnomad-vcf-test
Closed

feat: Address deletion and substitution bugs in gnomad_vcf_to_protein_variation.py#646
jarbesfeld wants to merge 4 commits intostagingfrom
gnomad-vcf-test

Conversation

@jarbesfeld
Copy link

closes #447

Not entirely sure why my reading frame change works, but would like to test with other known cases if possible

@jarbesfeld jarbesfeld self-assigned this Feb 5, 2026
@jarbesfeld jarbesfeld added bug Something isn't working priority:medium Medium priority labels Feb 5, 2026
Copy link
Member

@korikuzma korikuzma left a comment

Choose a reason for hiding this comment

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

Didn't review changes (just want to link to resources if they exist)

@jarbesfeld jarbesfeld requested a review from korikuzma February 5, 2026 20:03
cdk11a_e314del,
):
"""Test that deletion queries return correct response"""
# Reading Frame 0, Positive Strand (CA3250144726)

Choose a reason for hiding this comment

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

what does 0 vs 2 here mean?

Copy link
Author

Choose a reason for hiding this comment

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

@katie-perry This is the describing the reading frame. We used 0-indexing to describe the possible reading frames, but the options here would be 0,1,2 depending on where in the codon the deletion occurs

Choose a reason for hiding this comment

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

do we have any examples we could add for 1? Aside from that, the PR looks good to me

cdk11a_e314del,
):
"""Test that deletion queries return correct response"""
# Reading Frame 0, Positive Strand (CA3250144726)

Choose a reason for hiding this comment

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

do we have any examples we could add for 1? Aside from that, the PR looks good to me

Copy link

@katie-perry katie-perry left a comment

Choose a reason for hiding this comment

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

revoking my approval for now - I think we should explore a solution earlier in the workflow where we may not need different branching logic depending on the reading frame, if at all possible! I'm going to look into this today

@jarbesfeld
Copy link
Author

Closing this PR as the fix is being implemented in Katie's PR

@jarbesfeld jarbesfeld closed this Feb 12, 2026
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:medium Medium priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants