Repository navigation
sgkit zarr 3 migration - #1362
Conversation
Preserve the default Zarr format 2 storage, support current string codecs and store types, and replace the incompatible deprecated BGEN rechunker backend with Xarray and Dask. Update the lockfile, minimum-version CI, regression tests and documentation. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Drop bgen_to_zarr and rechunk_bgen following the maintainer decision. Preserve direct BGEN reading and legacy store unpacking, with tested manual read/chunk/save instructions while bio2zarr BGEN support is pending. Prominently document conversion via bio2zarr in the README, changelog, user guide and API reference. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Expose unpack_variables without the optional BGEN backend, preserve its old import path, and defer native reader imports until BGEN file access. Add blocked-backend import and unpacking regressions, preserve actionable dependency errors, and document the dependency-free API for issue 1331. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Delegate BGEN conversion to bio2zarr, remove the bgen extra and reader-only fixtures, and update dependency locking and wheel CI. Retain only dependency-free legacy unpacking and its compatibility import. Document the packed-store signature, reconstruction, missingness, lazy execution, and when not to unpack. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Clarify that BGEN reading and conversion are already removed, unlike the retained deprecated PLINK API, and point users to bio2zarr and legacy-store unpacking guidance. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Restore the existing general wheel smoke script by removing the added BGEN export/dependency assertions and synthetic unpacking example, as requested. Leave established behavioral tests unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Add the fsspec intersphinx inventory so StoreLike type annotations resolve FSMap. Use eager variable-length sample strings in the legacy packed-store fixture rather than unstable fixed-width UTF32 under Zarr format 3. Keep probability data chunked and warnings strict. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
fixed the docs! |
|
I haven't looked through the details of this, but I'm all for removing the legacy readers and trying to make a slimmer, more maintainable sgkit - so I'm +1 on this in principle |
I think this is reasonable for now. We've used zarr format v3 successfully where it's required for icechunk (using bio2zarr and vczstore). We should probably merge sgkit-dev/vcf-zarr-spec#42 now.
Me too - plink is deprecated so it can be removed too now. |
|
Okay! Can do, too! |
Move remaining validation callers to bio2zarr and document the conversion migration. Remove the PLINK extra, native dependency, fixtures and wheel smoke test. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
jeromekelleher
left a comment
jeromekelleher
left a comment
There was a problem hiding this comment.
LGTM - thanks @jpfeuffer !
|
@Billyzhang1229 @tomwhite would you like to have a look over? |
tomwhite
left a comment
tomwhite
left a comment
There was a problem hiding this comment.
LGTM
It's not clear to me why BGEN unpack_variables is needed - are there users who need it? Adding it introduces something else to maintain. Could it be a standalone script attached to a GH issue?
|
I was actually using it. The old converter/rechunker here produced files in a packed representation that you needed to use this unpacking to get e.g. dosages. see #1331 In the new bgen converter I actually don't use a packed representation so this should not be needed anymore. Unless we decide to support such a layout to save space with strictly diploid biallelic data. I kept it so people have a function to still unpack that old zarr representations. |
|
but yeah, happy to discuss in a follow-up issue. probably better suited there. |
|
I'm going to go ahead and merge them, thanks @jpfeuffer ! |
3 participants
Require Zarr Python ≥3 and xarray ≥2026.9.0, updating storage APIs, string codecs, dependency locks and CI accordingly. The default on-disk Zarr format remains 2; format 3 is opt-in.
Remove the legacy BGEN reader and converters (
read_bgen,bgen_to_zarr,rechunk_bgen), the deprecated PLINK reader/writer module (read_plink,plink_to_zarr,write_plink,zarr_to_plink), their optional extras and thecbgen,rechunkerandbed-readerdependencies.Use bio2zarr for conversion to Zarr and vcztools for export. Update migration guidance and remaining validation callers. BGEN conversion requires the work in sgkit-dev/bio2zarr#507; the corresponding exporter work is sgkit-dev/vcztools#440.
Retain
sgkit.io.bgen.unpack_variablesfor existing legacy packed datasets without requiring cbgen, preserving its original import path.changelog.rstapi.rst— N/A; no new public functions