Regions index #37
Regions index #37
Conversation
|
Tests are failing because we need https://github.com/tomwhite/bio2zarr/tree/variant-end (or similar). |
There was a problem hiding this comment.
This is fantastic, thanks @tomwhite! Some minor comments.
Should we get the variant_length added to vcf2zarr first (and maybe even compute the index, optionally, there)?
vcztools/regions.py
Outdated
|
|
||
| contig = root["variant_contig"] | ||
| pos = root["variant_position"] | ||
| end = root["variant_position_end"] |
There was a problem hiding this comment.
variant_length might be an easier name to understand and computing the end is simple from that. But that's a discussion for elsewhere
| c_end_idx - c_start_idx + 1, # num records | ||
| ) | ||
| ) | ||
| c_start_idx = c_end_idx + 1 |
There was a problem hiding this comment.
This looks just right! 🤩
Yes, we definitely want variant length/end in vcf2zarr first. (I see you've opened sgkit-dev/bio2zarr#278 for that - thanks.) I think we should compute the index in vcf2zarr - it's cheap to do so may not need to be optional. But we could wait a bit on that until we are sure everything's working with the index we've designed first. |
|
Thanks @tomwhite, this is a big step forward! |
Following discussion in sgkit-dev/vcf-zarr-spec#21