Skip to content

VCF support for RLE and fix to same-as-reference allele normalization to RLE #589

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.

Sign up for GitHub

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

Jump to bottom
Merged
theferrit32 merged 42 commits into from
Dec 15, 2025
Merged

VCF support for RLE and fix to same-as-reference allele normalization to RLE #589

theferrit32 merged 42 commits into from
Dec 15, 2025

Conversation

@theferrit32
Copy link
Contributor

@theferrit32 theferrit32 commented Nov 12, 2025 •
edited
Loading

Close #577
Close #587

@theferrit32 theferrit32 self-assigned this Nov 12, 2025
@jsstevenson jsstevenson requested review from jsstevenson and removed request for jsstevenson November 12, 2025 19:25
@theferrit32
Include sequence value in output vcf if the length of that sequence i…
c0042c3 
…s <= MAX_LITERAL_STATE_LENGTH, defaulted to 15
@theferrit32
Remove commented blocks
6df2ba2
@theferrit32
Update test name
460d6bb
@theferrit32
Add comments to cases in test_vrs_normalize.py
b7fa124
@theferrit32
Add make target clean-cassettes
576e4db
@theferrit32
Minor tweak to all_played checks in vcf annotator tests to make re-re…
70bd6e2 
…cording easier
@theferrit32
Ignore VRS and Python version header values in VCF tests
adf5f73
@theferrit32
Add missing cassette
2c5c1dd
@theferrit32
Don't fail test when vcr is being rewritten. Refresh cassettes
9a65b82
@theferrit32
add decode_compressed_response=True to vcr config
94ad735
@theferrit32
Add vcr_config to conftest.py. Delete vcr_support.py. Refresh cassettes
7723d5c
@theferrit32
Merge remote-tracking branch 'origin/main' into issue-577-VCF-RLE
6255779
@theferrit32
Reset casettes for RLE tests after merge from main
26eeba3
@jsstevenson jsstevenson mentioned this pull request Nov 20, 2025
Closed
@jsstevenson jsstevenson self-requested a review December 4, 2025 02:53
@theferrit32
Copy link
Contributor Author

theferrit32 commented Dec 4, 2025

Only thing left is addressing this comment
#589 (comment)

korikuzma
korikuzma reviewed Dec 4, 2025
View reviewed changes
Copy link
Contributor

@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.

Submitting review for VCF code now. Going to be looking at the normalize fix next.

@korikuzma korikuzma self-requested a review December 4, 2025 18:21
korikuzma
korikuzma reviewed Dec 4, 2025
View reviewed changes
korikuzma
korikuzma reviewed Dec 5, 2025
View reviewed changes
Copy link
Contributor

@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.

Tests LGTM

@theferrit32
Copy link
Contributor Author

theferrit32 commented Dec 12, 2025

VCF changes after fixing the max seq limit logic, and bumping it up to 50bp to match the rle_seq_limit default elsewhere

vrs-python/src/ga4gh/vrs/normalize.py

Line 86 in 9462037

def _normalize_allele(input_allele, data_proxy, rle_seq_limit=50): 

git diff --word-diff-regex='[^;]+' -- **/*.vcf

tests/extras/data/test_vcf_expected_altsonly_output.vcf
chr19 289464 . T TCACGCCTGTAATCC 50 PASS platforms=4;platformnames=Illumina,PacBio,CG,10X;datasets=4;datasetnames=HiSeqPE300x,CCS15kb_20kb,CGnormal,10XChromiumLR;callsets=6;callsetnames=HiSeqPE300xGATK,CCS15kb_20kbGATK4,CGnormal,HiSeqPE300xfreebayes,CCS15kb_20kbDV,10XLRGATK;datasetsmissingcall=IonExome,SolidSE75bp;callable=CS_HiSeqPE300xGATK_callable,CS_10XLRGATK_callable,CS_CCS15kb_20kbGATK4_callable,CS_CGnormal_callable,CS_HiSeqPE300xfreebayes_callable;filt=CS_CCS15kb_20kbDV_filt,CS_CCS15kb_20kbGATK4_filt;VRS_Allele_IDs=ga4gh:VA.ySvDptXfHB_9WEfu78v32DzBXJfwGgO7;VRS_Starts=289464;VRS_Ends=289466;[-VRS_States-]{+VRS_States=CACGCCTGTAATCCCA+};VRS_Lengths=.;VRS_RepeatSubunitLengths=. GT:PS:DP:ADALL:AD:GQ 0/1:.:518:94,98:116,137:785

tests/extras/data/test_vcf_expected_output.vcf
chr19 289464 . T TCACGCCTGTAATCC 50 PASS platforms=4;platformnames=Illumina,PacBio,CG,10X;datasets=4;datasetnames=HiSeqPE300x,CCS15kb_20kb,CGnormal,10XChromiumLR;callsets=6;callsetnames=HiSeqPE300xGATK,CCS15kb_20kbGATK4,CGnormal,HiSeqPE300xfreebayes,CCS15kb_20kbDV,10XLRGATK;datasetsmissingcall=IonExome,SolidSE75bp;callable=CS_HiSeqPE300xGATK_callable,CS_10XLRGATK_callable,CS_CCS15kb_20kbGATK4_callable,CS_CGnormal_callable,CS_HiSeqPE300xfreebayes_callable;filt=CS_CCS15kb_20kbDV_filt,CS_CCS15kb_20kbGATK4_filt;VRS_Allele_IDs=ga4gh:VA.nqqTUy-a2gssemOmJb4CJv-HNuFAmGrO,ga4gh:VA.ySvDptXfHB_9WEfu78v32DzBXJfwGgO7;VRS_Starts=289463,289464;VRS_Ends=289464,289466;[-VRS_States=T,-]{+VRS_States=T,CACGCCTGTAATCCCA+};VRS_Lengths=1,.;VRS_RepeatSubunitLengths=1,. GT:PS:DP:ADALL:AD:GQ 0/1:.:518:94,98:116,137:785

theferrit32 and others added 5 commits December 12, 2025 16:38
@theferrit32
Update vcf.py to replace MAX_LITERAL_STATE_LENGTH with a RLE_SEQ_LIMI…
f3ce393 
…T specific to RLE that defaults to 50, and excludes sequence vals from INFO if over that
@theferrit32 @korikuzma
Apply suggestions from code review
83f9296 
Co-authored-by: Kori Kuzma <korikuzma@gmail.com>
@theferrit32
Use enumerated sequence types in vcf.py
a52ec24
@theferrit32 @korikuzma
Update src/ga4gh/vrs/normalize.py
938e3e0 
Co-authored-by: Kori Kuzma <korikuzma@gmail.com>
@theferrit32
Fix formatting
2234438
korikuzma
korikuzma reviewed Dec 15, 2025
View reviewed changes
korikuzma
korikuzma approved these changes Dec 15, 2025
View reviewed changes
Copy link
Contributor

@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.

Fantastic work @theferrit32 🚀

@korikuzma korikuzma mentioned this pull request Dec 15, 2025
Open
@theferrit32 theferrit32 merged commit ded576f into main Dec 15, 2025
16 checks passed
@theferrit32 theferrit32 deleted the issue-577-VCF-RLE branch December 15, 2025 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@korikuzma korikuzma korikuzma approved these changes

@ahwagner ahwagner Awaiting requested review from ahwagner

@jsstevenson jsstevenson Awaiting requested review from jsstevenson

@larrybabb larrybabb Awaiting requested review from larrybabb

Assignees

@theferrit32 theferrit32

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Same-as-reference Alleles should have a ReferenceLengthExpression state Add RLE params to VRS-annotated VCFs

5 participants

@theferrit32 @larrybabb @jsstevenson @korikuzma