Add states() method to return actual genotype states as characters

[hyanwong]

Description

Adds variant.genotype_values(). See tskit-dev/tsinfer#739

PR Checklist:

• Tests that fully cover new/changed functionality.
• Documentation including tutorial content if appropriate.
• Changelogs, if there are API changes.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.72%. Comparing base (d6b2303) to head (efa8d08).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2617   +/-   ##
=======================================
  Coverage   89.72%   89.72%           
=======================================
  Files          29       29           
  Lines       31578    31589   +11     
  Branches     6117     6121    +4     
=======================================
+ Hits        28333    28344   +11     
  Misses       1853     1853           
  Partials     1392     1392           

Flag	Coverage Δ
c-tests	86.55% <ø> (ø)
lwt-tests	80.78% <ø> (ø)
python-c-tests	88.98% <ø> (ø)
python-tests	99.01% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
python/tskit/genotypes.py	99.15% <100.00%> (+0.08%)	⬆️

[hyanwong]

I'm not sure why this is failing CI: everything is covered AFAICS.

Ready to merge, I reckon, pending approval of the name of the function.

[jeromekelleher]

LGTM - not sure about the name though.

[jeromekelleher]

Also not clear that an Object array is what people would want. Would it be better to have a "missing data symbol" or something like we do for haplotypes?

[hyanwong]

Also not clear that an Object array is what people would want. Would it be better to have a "missing data symbol" or something like we do for haplotypes?

ISWYM, but using any particular character will mean that we can't use that for an allele type (which we might conceivably want to do). This is only going to bite for missing data, so I don't think it's too bad TBH.

[jeromekelleher]

Hmm - how about writing some client code for this with and without missing data? Something like the verify function in tsinfer would be a good one. We definitely don't want to have to have different code paths for dtypes with and without missing data.

We use "N" as the default missing data character elsewhere and its fine. We should be able to use this function to compare easily with e.g. alignments.

Is the np.array(list(ts.alignments()) equal to np.array(var.states() for var in ts.variants()).T with and without missing data?

[benjeffery]

@hyanwong Would you like this to be in 0.5.4?

[hyanwong]

Ah, good point. There's no hurry from my PoV. Is it just a question of my changing the name? I can do that easily enough.

[jeromekelleher]

I don't think we should hurry this in as we don't want to get stuck with Object arrays unless we have to. They have subtly different semantics to arrays of unicode.

We should have some example client code for this before committing ourselves to a particular representation.

[hyanwong]

Sure. I have updated this so it should never return an object array anyway. But no hurry to get this in. It was to help @szhan and others who spent a fair bit of time comparing integer arrays between tree sequence before realising that they weren't actually comparable.

[hyanwong]

I think this also fixes #2181 . The name was changed to .states() after discussion with @jeromekelleher, but in #2181 he also suggested genotypes_as_alleles?