fix_vcf_ploidies: bgzip must stay a filter, not take a filename - #1989
Merged
Conversation
14d0589 ("log input vcf path") added raw_out_path as a positional argument to the bgzip call while leaving cactus_call's infile=/outfile= redirects in place. The intent was to get the path into the logged command line, but a filename argument switches bgzip from a stdin/stdout filter to compressing in place, and the two cannot be combined. cactus_call stages stdout under outfile + '.part' and only renames it into place once the command exits cleanly, so out_vcf_path does not exist when bgzip runs and bgzip takes the in-place path: 1. bgzip writes the real compressed VCF to raw_out_path + '.gz', which IS out_vcf_path, and deletes raw_out_path. It exits 0. 2. cactus_call sees success and os.replace()s its empty .part file over out_vcf_path, discarding what bgzip just wrote. 3. os.remove(raw_out_path) raises FileNotFoundError, because bgzip took it. Step 3 is the only reason this is loud. By then the output is already gone, so anything that swallowed that exception would ship an empty VCF with exit 0. Only reachable when the VCF actually needs ploidy padding: the scan above returns early via shutil.copyfile when every sample has one ploidy throughout, so a run whose samples are uniformly diploid never gets here. Mixed ploidy -- chrY alongside the autosomes -- is the common case that does. Reverts to the form d521880 introduced and that minigraph_gfa_to_pansn uses: no filename, infile/outfile only. Verified against the bgzip in bin/: with a filename and no pre-existing .gz it deletes the input and exits 0 (the failure above); as a filter it leaves the input alone, writes 183 bytes for a 3-line test VCF, and round-trips byte-identically through bgzip -dc. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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.
14d05895("log input vcf path", merged in #1982) addedraw_out_pathas a positional argument to thebgzipcall infix_vcf_ploidies, while leavingcactus_call'sinfile=/outfile=redirects in place. A filename argument switchesbgzipfrom a stdin/stdout filter to compressing in place, and the two cannot be combined.cactus_callstages stdout underoutfile + '.part'and renames it into place only on clean exit, soout_vcf_pathdoes not exist whenbgzipruns, andbgziptakes the in-place path:bgzipwrites the real compressed VCF toraw_out_path + '.gz'— which isout_vcf_path— and deletesraw_out_path. It exits 0.cactus_callsees success andos.replace()s its empty.partfile overout_vcf_path, discarding whatbgzipjust wrote.os.remove(raw_out_path)raisesFileNotFoundError, becausebgziptook it.Step 3 is the only reason this fails loudly. By then the output is already gone, so anything that swallowed that exception would ship an empty VCF with exit 0.
Reachability
Only when the VCF actually needs ploidy padding. The scan above returns early via
shutil.copyfilewhen every sample has one ploidy throughout, so a uniformly-diploid run never reaches this line. Mixed ploidy — chrY alongside the autosomes — is the common case that does.Fix
Reverts to the form
d5218801introduced, and thatminigraph_gfa_to_pansnuses: no filename,infile/outfileonly.Verification
Against the
bgzipinbin/:.gz: deletes the input, exits 0 — the failure abovebgzip -dcObserved in the wild as repeated
FileNotFoundError: .../clip.{raw,bub,wave}.ploidy.vcffromvcf_caton a 30-way HPRC run.