Split out from the gvl.concat work (#334, PR #339). Deferred deliberately: an .svar2 fixture needs test infrastructure the concat plan never scoped.
What is missing
gvl.concat implements the .svar2 backend (_concat_svar2_ranges in python/genvarloader/_dataset/_concat.py), but no test exercises it. The concat test suite covers PGEN/VCF on both axes, .svar on both axes, and tracks/annot tracks — but the entire svar2 branch ships untested.
The blocker is fixture infrastructure: tests/_builders/case.py's Case dataclass has pgen_path and svar_path but no svar2_path, so there is no session-scoped .svar2 store to shard and merge.
Why it was not treated as blocking
A review verified the svar2 merge code against the real on-disk format rather than against tests, and found it consistent:
- filenames (
vk_snp_range.npy, vk_indel_range.npy, dense_snp_range.npy, dense_indel_range.npy, sample_cols.npy, svar2_meta.json)
- the raw-memmap vs
np.save split — the four range arrays are written headerless via gather_fixed/link_or_copy_buffered; sample_cols goes through np.save, matching the reader's np.load
- the 16-byte record stride and
(R, S, P, 2) C-order slot space
- the four
["shape"] updates in svar2_meta.json, with "ploidy" preserved
- the reader derives
R from dense_snp_range.shape[0] and S from vk_snp_range.shape[1], both of which the merge sets correctly
Worth noting the region-axis dense_* gather also fixes a latent block-concat bug that was present in the original plan's np.concatenate approach.
So: consistent-by-inspection, but unverified by execution.
What to do
- Add an
.svar2 store to the shared synthetic case (svar2_path on Case, built alongside the existing .svar).
- Add concat fixtures sharding it by region and by sample.
- Assert against a single-shot
gvl.write oracle, mirroring the existing svar tests.
- Include an interleaved sample split (e.g.
[s0, s2] + [s1]). This matters: the plain fixtures split samples alphabetically, so after re-sorting they collapse to a block layout — the exact case a wrong slot ordering would also pass. Only an interleaved split proves the order= handling is right for svar2.
Deferred minors from the same review
Worth folding in if someone is already in this code:
svar_link/svar2_link are copied verbatim into merged metadata, but relative_path resolves against the dataset dir, so it is stale for a merged dataset written elsewhere. Resolution silently falls back to absolute_path, so tests pass — a later tree move breaks it. Recompute the link against the new destination.
_gather_svar_offsets has unused n_src_slots/n locals, re-reads each input's offsets file once per plane (2x IO), and np.stack(planes).tobytes() adds two full-size copies on top of the permitted materialization. Writing each plane's bytes separately would halve peak memory inside that carve-out.
Split out from the
gvl.concatwork (#334, PR #339). Deferred deliberately: an.svar2fixture needs test infrastructure the concat plan never scoped.What is missing
gvl.concatimplements the.svar2backend (_concat_svar2_rangesinpython/genvarloader/_dataset/_concat.py), but no test exercises it. The concat test suite covers PGEN/VCF on both axes,.svaron both axes, and tracks/annot tracks — but the entire svar2 branch ships untested.The blocker is fixture infrastructure:
tests/_builders/case.py'sCasedataclass haspgen_pathandsvar_pathbut nosvar2_path, so there is no session-scoped.svar2store to shard and merge.Why it was not treated as blocking
A review verified the svar2 merge code against the real on-disk format rather than against tests, and found it consistent:
vk_snp_range.npy,vk_indel_range.npy,dense_snp_range.npy,dense_indel_range.npy,sample_cols.npy,svar2_meta.json)np.savesplit — the four range arrays are written headerless viagather_fixed/link_or_copy_buffered;sample_colsgoes throughnp.save, matching the reader'snp.load(R, S, P, 2)C-order slot space["shape"]updates insvar2_meta.json, with"ploidy"preservedRfromdense_snp_range.shape[0]andSfromvk_snp_range.shape[1], both of which the merge sets correctlyWorth noting the region-axis
dense_*gather also fixes a latent block-concat bug that was present in the original plan'snp.concatenateapproach.So: consistent-by-inspection, but unverified by execution.
What to do
.svar2store to the shared synthetic case (svar2_pathonCase, built alongside the existing.svar).gvl.writeoracle, mirroring the existing svar tests.[s0, s2]+[s1]). This matters: the plain fixtures split samples alphabetically, so after re-sorting they collapse to a block layout — the exact case a wrong slot ordering would also pass. Only an interleaved split proves theorder=handling is right for svar2.Deferred minors from the same review
Worth folding in if someone is already in this code:
svar_link/svar2_linkare copied verbatim into merged metadata, butrelative_pathresolves against the dataset dir, so it is stale for a merged dataset written elsewhere. Resolution silently falls back toabsolute_path, so tests pass — a later tree move breaks it. Recompute the link against the new destination._gather_svar_offsetshas unusedn_src_slots/nlocals, re-reads each input's offsets file once per plane (2x IO), andnp.stack(planes).tobytes()adds two full-size copies on top of the permitted materialization. Writing each plane's bytes separately would halve peak memory inside that carve-out.