Use of int8 for variant_contig results in integer overflow with fragmented reference genomes

[timothymillar]

Many (non human) reference genomes contain 1000s of contigs that have not been assembled into full chromosomes.
Currently the variant_contig array is hard coded as int8 (line) which results in integer overflow making it impossible to join variants to their contig.

[hammer]

We should probably revisit how we're parsing and representing contig metadata more generally, as noted in https://github.com/pystatgen/sgkit/issues/464 as well.

@timothymillar do you have any example VCFs to share that overflow the int8 limit? It's trivial to manually construct such a VCF but I generally prefer to have real world data in our tests.

[timothymillar]

@hammer agreed, real data always hits more edge cases. I'll look into an example VCF for this.

For reference I ran into this issue when using the Red5 kiwifruit genome.

[tomwhite]

We use int16 for bgen and PLINK, so we should probably just change VCF to be the same.

[jeromekelleher]

We should make it an option to specify the dtype for variant_contig probably - even int16 will overflow sometimes. There are lots of VCFs out there with huge numbers of contigs.

Although, I guess this is the sort of thing we should be able to query the IO library for ("how many contigs are there" should be efficiently computable on any indexed VCF), so we should be able to automatically detect the minimal dtype. Even then though, I suppose people might want to manually specify the dtype, for their own reasons.

[alxsimon]

Is there a workaround at the moment? Working with a non model species with > 50000 contigs

[jeromekelleher]

Nice - great to see you pushing the limits here @alxsimon! @tomwhite any thoughts on how we should address this?

[tomwhite]

I'm afraid I can't think of a workaround for this. The fix that @jeromekelleher sketched out above should be fairly straightforward though, so I'll work on a PR to fix it.

[alxsimon]

Thanks @tomwhite and @jeromekelleher, I'll wait for the upstream fix.

[alxsimon]

Quick and dirty workaround to reimport the contig names from the vcf, in case someone else is looking for a way to do this in the meantime.

from cyvcf2 import VCF

variant_contig_name = np.empty(ds.dims['variants'], dtype="O")
variant_position = np.empty(ds.dims['variants'], dtype="i4")
for idx, variant in enumerate(VCF(bcf_file)):
    variant_contig_name[idx] = variant.CHROM
    variant_position[idx] = variant.POS

ds = ds.merge(xr.DataArray(variant_contig_name, coords=ds.coords, dims=['variants'], name='variant_contig_name'))

[tomwhite]

Nice!

I've created a fix in #667. Hopefully we can get that merged soon for you to use.