Implement allele counting methods #2393

hyanwong · 2022-07-08T10:19:32Z

Description

Add variant.num_missing, variant.counts(), and variant.frequencies(). Ping @szhan - is this useful for #2384 ?

Fixes #2390

PR Checklist:

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

hyanwong · 2022-07-08T10:19:53Z

No tests yet. Is the interface right @benjeffery ?

szhan · 2022-07-08T10:25:17Z

Ah, yes, they are useful for #2384.

szhan · 2022-07-08T10:28:43Z

They are also useful for some information-theoretic imputation quality metrics that I want to implement soon.

codecov · 2022-07-08T10:31:43Z

Codecov Report

Merging #2393 (812af67) into main (f41eddc) will decrease coverage by 0.00%.
The diff coverage is n/a.

❗ Current head 812af67 differs from pull request most recent head 22c10aa. Consider uploading reports for the commit 22c10aa to get more accurate results

@@            Coverage Diff             @@
##             main    #2393      +/-   ##
==========================================
- Coverage   93.34%   93.34%   -0.01%     
==========================================
  Files          28       28              
  Lines       26988    26982       -6     
  Branches     1246     1245       -1     
==========================================
- Hits        25193    25187       -6     
  Misses       1761     1761              
  Partials       34       34

Flag	Coverage Δ
c-tests	`92.26% <0.00%> (ø)`
lwt-tests	`89.05% <0.00%> (ø)`
python-c-tests	`71.01% <0.00%> (-0.02%)`	⬇️
python-tests	`98.96% <0.00%> (-0.01%)`	⬇️

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

Impacted Files	Coverage Δ
python/tskit/combinatorics.py	`99.36% <0.00%> (-0.01%)`	⬇️
python/tskit/trees.py	`98.66% <0.00%> (ø)`
python/tskit/__init__.py	`100.00% <0.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f41eddc...22c10aa. Read the comment docs.

benjeffery · 2022-07-08T12:26:39Z

No tests yet. Is the interface right @benjeffery ?

Interface looks good to me, although I'm not sure about raising an error on there being zero samples, what about an empty dict as the return in that case? That's what counts would do.

It would also be nice to reuse counts in frequencies.

hyanwong · 2022-07-08T12:37:52Z

No tests yet. Is the interface right @benjeffery ?

Interface looks good to me, although I'm not sure about raising an error on there being zero samples, what about an empty dict as the return in that case?

Yeah, I wasn't sure about that. Another possibility would be to return NaN?

That's what counts would do.

Would it. That seems a mistake to me. I think it should return all the alleles with a count of zero?

It would also be nice to reuse counts in frequencies.

I could do that. It seemed a bit more work, but probably about the same.

hyanwong · 2022-07-08T12:40:39Z

Would it. That seems a mistake to me. I think it should return all the alleles with a count of zero?

Oh yes, I think I coded it wrongly. I suspect it should return a value for everything in variant.alleles?

hyanwong · 2022-07-08T14:02:06Z

I coded this to raise a warning if frequencies are calculated on variant with all missing samples or no samples, but output NaN (which is mathematically the correct thing, I think). Is this right, and should I create a separate logger instance in this case @benjeffery ?

petrelharp · 2022-07-09T16:44:13Z

This looks very nice. Note: this overlaps with #504.

benjeffery

There's a mistake in the denominator for frequencies, I think!

python/tskit/genotypes.py

hyanwong · 2022-07-12T11:58:56Z

Hmm, an exciting segfault here. WTF?

Edit - failing due to #2400

benjeffery · 2022-07-14T11:07:25Z

@hyanwong Did you want to get this in for 0.5.1?

hyanwong · 2022-07-14T11:10:09Z

Maybe release 5.1 first, as we might want to think about how it interacts with @petrelharp's suggestions in #504, and nothing is waiting on this PR.

If @szhan wants to use it for #2384, he can simply wait until we decide on the right API.

jeromekelleher

LGTM, implementation comment. Should also consider options in the stdlib for representating frequency mapping

python/tskit/genotypes.py

hyanwong · 2022-07-15T14:38:16Z

Should also consider options in the stdlib for representating frequency mapping

I had a hunt around and couldn't see anything terribly obvious.

jeromekelleher

LGTM. Needs a few explicit tests, as it's quite hard to see what's really being tested when checking against the ts_fixture.

python/CHANGELOG.rst

python/tests/test_genotypes.py

python/tskit/genotypes.py

jeromekelleher

LGTM

hyanwong · 2022-07-19T18:35:43Z

Can we merge this now? It would be useful for @szhan I think (and @benjeffery is on holiday)

jeromekelleher · 2022-07-20T08:55:37Z

Merging - I'm assuming the changes Ben requested have been made? (Not obvious what they were here, I came in late to the conversation)

To make PR mergable

petrelharp · 2022-07-20T19:13:48Z

python/tskit/genotypes.py

+            bincounts = np.bincount(self.genotypes)
+            for i, allele in enumerate(self.alleles):
+                counts[allele] = bincounts[i] if i < len(bincounts) else 0


Late here, but I think you could just do this?

bincounts = np.bincount(self.genotypes, minlength=self.num_alleles) for i, allele in enumerate(self.alleles): counts[allele] = bincounts[i]

petrelharp · 2022-07-20T19:18:02Z

This is excellent!

hyanwong force-pushed the variant-counts branch 2 times, most recently from 6d67bde to ecae7cf Compare July 8, 2022 14:00

hyanwong force-pushed the variant-counts branch 3 times, most recently from 177a999 to b5c301c Compare July 8, 2022 15:52

hyanwong requested a review from benjeffery July 9, 2022 13:10

hyanwong marked this pull request as ready for review July 9, 2022 13:10

benjeffery previously requested changes Jul 12, 2022

View reviewed changes

python/tskit/genotypes.py Outdated Show resolved Hide resolved

python/tskit/genotypes.py Show resolved Hide resolved

hyanwong force-pushed the variant-counts branch from b5c301c to 14a2295 Compare July 12, 2022 11:41

hyanwong force-pushed the variant-counts branch from 14a2295 to 246be85 Compare July 14, 2022 11:06

hyanwong force-pushed the variant-counts branch from 246be85 to 22c10aa Compare July 14, 2022 15:25

jeromekelleher reviewed Jul 15, 2022

View reviewed changes

python/tskit/genotypes.py Outdated Show resolved Hide resolved

python/tskit/genotypes.py Outdated Show resolved Hide resolved

hyanwong force-pushed the variant-counts branch 2 times, most recently from 4d9aec2 to 1dcad55 Compare July 15, 2022 13:08

hyanwong requested a review from benjeffery July 15, 2022 14:18

hyanwong force-pushed the variant-counts branch from 1dcad55 to 97e345a Compare July 15, 2022 14:36

jeromekelleher reviewed Jul 18, 2022

View reviewed changes

python/CHANGELOG.rst Outdated Show resolved Hide resolved

python/tests/test_genotypes.py Show resolved Hide resolved

python/tskit/genotypes.py Outdated Show resolved Hide resolved

python/tskit/genotypes.py Show resolved Hide resolved

hyanwong force-pushed the variant-counts branch from 97e345a to 1fee72a Compare July 18, 2022 21:49

Implement allele counting methods

abd4380

hyanwong force-pushed the variant-counts branch from 1fee72a to abd4380 Compare July 19, 2022 09:13

jeromekelleher approved these changes Jul 19, 2022

View reviewed changes

jeromekelleher added the AUTOMERGE-REQUESTED Ask Mergify to merge this PR label Jul 20, 2022

mergify bot merged commit 33df3ca into tskit-dev:main Jul 20, 2022

mergify bot removed the AUTOMERGE-REQUESTED Ask Mergify to merge this PR label Jul 20, 2022

petrelharp reviewed Jul 20, 2022

View reviewed changes

petrelharp mentioned this pull request Jul 20, 2022

tweak to bincount #2425

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement allele counting methods #2393

Implement allele counting methods #2393

hyanwong commented Jul 8, 2022 •

edited

Loading

hyanwong commented Jul 8, 2022

szhan commented Jul 8, 2022

szhan commented Jul 8, 2022

codecov bot commented Jul 8, 2022 •

edited

Loading

benjeffery commented Jul 8, 2022

hyanwong commented Jul 8, 2022

hyanwong commented Jul 8, 2022

hyanwong commented Jul 8, 2022 •

edited

Loading

petrelharp commented Jul 9, 2022

benjeffery left a comment

hyanwong commented Jul 12, 2022 •

edited

Loading

benjeffery commented Jul 14, 2022

hyanwong commented Jul 14, 2022 •

edited

Loading

jeromekelleher left a comment

hyanwong commented Jul 15, 2022

jeromekelleher left a comment

jeromekelleher left a comment

hyanwong commented Jul 19, 2022

jeromekelleher commented Jul 20, 2022

petrelharp Jul 20, 2022 •

edited

Loading

petrelharp commented Jul 20, 2022

Implement allele counting methods #2393

Implement allele counting methods #2393

Conversation

hyanwong commented Jul 8, 2022 • edited Loading

Description

PR Checklist:

hyanwong commented Jul 8, 2022

szhan commented Jul 8, 2022

szhan commented Jul 8, 2022

codecov bot commented Jul 8, 2022 • edited Loading

Codecov Report

benjeffery commented Jul 8, 2022

hyanwong commented Jul 8, 2022

hyanwong commented Jul 8, 2022

hyanwong commented Jul 8, 2022 • edited Loading

petrelharp commented Jul 9, 2022

benjeffery left a comment

Choose a reason for hiding this comment

hyanwong commented Jul 12, 2022 • edited Loading

benjeffery commented Jul 14, 2022

hyanwong commented Jul 14, 2022 • edited Loading

jeromekelleher left a comment

Choose a reason for hiding this comment

hyanwong commented Jul 15, 2022

jeromekelleher left a comment

Choose a reason for hiding this comment

jeromekelleher left a comment

Choose a reason for hiding this comment

hyanwong commented Jul 19, 2022

jeromekelleher commented Jul 20, 2022

petrelharp Jul 20, 2022 • edited Loading

Choose a reason for hiding this comment

petrelharp commented Jul 20, 2022

hyanwong commented Jul 8, 2022 •

edited

Loading

codecov bot commented Jul 8, 2022 •

edited

Loading

hyanwong commented Jul 8, 2022 •

edited

Loading

hyanwong commented Jul 12, 2022 •

edited

Loading

hyanwong commented Jul 14, 2022 •

edited

Loading

petrelharp Jul 20, 2022 •

edited

Loading