Surface mount-time startup errors without stack traces#12
Merged
jeromekelleher merged 2 commits intosgkit-dev:mainfrom May 7, 2026
Merged
Surface mount-time startup errors without stack traces#12jeromekelleher merged 2 commits intosgkit-dev:mainfrom
jeromekelleher merged 2 commits intosgkit-dev:mainfrom
Conversation
Running mount-plink against an unsupported VCZ (e.g. multi-allelic) used to dump two tracebacks: one from the plink-server subprocess crashing on the unhandled ValueError out of generate_bim, and one from Click on the parent's uncaught OSError after the connect deadline expired. _server_main now wraps its body so startup failures log a single ERROR line (visible by default) plus a DEBUG traceback (only at -v), then return cleanly. PlinkClient detects an exited subprocess in both the pre-connect retry loop and the post-handshake failure path, surfacing a single OSError(EIO, "plink-server exited during startup; see log above for details"). The CLI's handle_exception decorator catches OSError too, so the user sees one Error: line instead of a Python traceback.
vcztools' BedEncoder refuses any reader with set_variant_filter() configured, so the bcftools-style filters mount-plink advertises (--max-alleles, --types, --include, ...) all failed at first connection with NotImplementedError. The error message biofuse prints on multi-allelic input even points users at --max-alleles 2 as the workaround, but it didn't work. _server_main now calls reader.materialise_variant_filter() between make_reader_from_options and _ServerSession. The call is a no-op when no filter is configured; otherwise it walks the chunk plan, evaluates the filter, and replaces (variant_filter, plan) with a fixed plan over the surviving variants — exactly what BedEncoder expects. _ServerSession.bed_size also switches from reader.num_variants (raw store count) to variant_counts_per_chunk().sum() (planned count) so .bed and .bim agree once a filter has been baked in. Tests cover --max-alleles 2 end-to-end via the CLI and through PlinkClient.start(reader_options=...) on the multi-allelic fixture; both assert .bim line count drops to fx_multiallelic_vcz.num_biallelic_sites (1309 vs 1382). VczFixture grows a num_biallelic_sites field computed at fixture-build time from variant_allele via zarr, mirroring vcztools' own _check_biallelic acceptance rule.
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
1 participant
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.
Running mount-plink against an unsupported VCZ (e.g. multi-allelic) used to dump two tracebacks: one from the plink-server subprocess crashing on the unhandled ValueError out of generate_bim, and one from Click on the parent's uncaught OSError after the connect deadline expired.