Repository navigation
fix: stream VCF_TO_CSV instead of retaining every record - #331
Merged
kubranarci merged 1 commit intoOct 5, 2026
Merged
Conversation
Discover optional INFO columns in one pass, then reopen the VCF and write one CSV row at a time. Peak memory no longer grows with record count, and the CSV contract stays byte-identical. Fixes adamrtalbot/hap-rs#89
adamrtalbot
added a commit
to adamrtalbot/variantbenchmarking
that referenced
this pull request
Oct 4, 2026
bin/vcf_to_csv.py: two streaming passes, bounded memory. Fixes adamrtalbot/hap-rs#89. Keep nf-core#88 open until executor kill reason is verified. Upstream: nf-core#331.
|
❌ nf-test failed with latest Nextflow versionNote Tests with Nextflow's latest version failed but it will not cause a CI workflow failure.
See the full run for details. |
kubranarci
reviewed
Oct 5, 2026
kubranarci
left a comment
kubranarci
left a comment
Contributor
There was a problem hiding this comment.
@VictorDidier is the developer of this script. I guess the changes are good to go but I will ask for his approval.
Contributor
|
Thanks @adamrtalbot for the fix! |
This was
linked to
issues
Oct 5, 2026
|
Thanks @adamrtalbot make a PR for this so fast! I think this two-pass approach is the best solution if it is important to maintain the same exact CSV output (maybe @kubranarci or @VictorDidier could give input on this). If not, have you considered a single-pass solution where all extra columns (e.g. SVTYPE) are represented in the CSV, regardless of their presence in the VCF? This would be faster and the output CSV would have the same format regardless of input. |
Author
Honestly, I just made it match as closely as I could! My gut feeling is to merge now to fix the expanding memory, then open a fresh PR for any further improvements. |
Author
|
I have to admit I didn't see the open issue, I found the issue myself while analysing long read VCFs. |
Contributor
this module doesnt have a csv output https://github.com/nf-core/variantbenchmarking/blob/dev/modules/local/plots/svlen_dist/main.nf, it uses CSV input. As long as it produces understandable plots I am eager to test. |
Contributor
I guess there were many people affected from this. My tests on big-long vcf files is limited to be honest. |
Sound good to me, just wanted to lift this as something to consider. Happy to go with this solution :) |
kubranarci
approved these changes
Oct 5, 2026
Contributor
|
ok then lets merge this to dev, please let me know if it works fine for you as well @pontushojer |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
bin/vcf_to_csv.pykept every(row, info_dict)until EOF, so peak memory grew with the number of records. It now streams the VCF once to learn the header and the five optional-column flags (SUPP_VEC,SUPP,type_inferred,SVTYPE,SVLEN), reopens the same path, and writes one CSV row at a time.The CLI,
parse_info_field,extract_gt_from_sample, and the CSV dialect are unchanged. Output is byte-identical to the previous converter. No workflow, resource, or.nf-core.ymlchanges.Related: adamrtalbot/hap-rs#89
PR checklist
Local check: byte-identical against the previous converter, and
nf-test test modules/local/custom/vcf_to_csv/tests/main.nf.test --profile=+docker(2 passed, existing snapshot unchanged). No new test file; the module snapshot already covers the output contract.nf-core pipelines lint).nextflow run . -profile test,docker --outdir <OUTDIR>).nextflow run . -profile debug,test,docker --outdir <OUTDIR>).docs/usage.mdis updated.docs/output.mdis updated.CHANGELOG.mdis updated.README.mdis updated (including new tool citations and authors/contributors).Docs, changelog, and README are unchanged because the CSV contract is the same.