tskit-dev / tskit

Population-scale genomics
MIT License
147 stars 70 forks source link

Two locus user-facing python ld matrix API #2906

Closed lkirk closed 4 months ago

lkirk commented 5 months ago

Description

This is an implementation of the user-facing python API for ld_matrix as discussed in #2829

I've added some tests that cover most edge cases. I think I'd like to add some tests that store literal matricies, but I wanted to get eyes on this before I added those final tests.

PR Checklist:

lkirk commented 5 months ago

cc @apragsdale

codecov[bot] commented 5 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (04e04aa) 89.73% compared to head (2caf278) 89.75%.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/tskit-dev/tskit/pull/2906/graphs/tree.svg?width=650&height=150&src=pr&token=7RoAMA56V2&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev)](https://app.codecov.io/gh/tskit-dev/tskit/pull/2906?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev) ```diff @@ Coverage Diff @@ ## main #2906 +/- ## ========================================== + Coverage 89.73% 89.75% +0.01% ========================================== Files 30 30 Lines 30360 30399 +39 Branches 5903 5913 +10 ========================================== + Hits 27245 27285 +40 Misses 1782 1782 + Partials 1333 1332 -1 ``` | [Flag](https://app.codecov.io/gh/tskit-dev/tskit/pull/2906/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev) | Coverage Δ | | |---|---|---| | [c-tests](https://app.codecov.io/gh/tskit-dev/tskit/pull/2906/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev) | `86.13% <100.00%> (+<0.01%)` | :arrow_up: | | [lwt-tests](https://app.codecov.io/gh/tskit-dev/tskit/pull/2906/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev) | `80.78% <ø> (ø)` | | | [python-c-tests](https://app.codecov.io/gh/tskit-dev/tskit/pull/2906/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev) | `67.71% <5.40%> (-0.15%)` | :arrow_down: | | [python-tests](https://app.codecov.io/gh/tskit-dev/tskit/pull/2906/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev) | `98.96% <100.00%> (+<0.01%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files](https://app.codecov.io/gh/tskit-dev/tskit/pull/2906?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev) | Coverage Δ | | |---|---|---| | [c/tskit/trees.c](https://app.codecov.io/gh/tskit-dev/tskit/pull/2906?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev#diff-Yy90c2tpdC90cmVlcy5j) | `90.21% <100.00%> (+0.02%)` | :arrow_up: | | [python/tskit/trees.py](https://app.codecov.io/gh/tskit-dev/tskit/pull/2906?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev#diff-cHl0aG9uL3Rza2l0L3RyZWVzLnB5) | `98.76% <100.00%> (+0.02%)` | :arrow_up: | ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/tskit-dev/tskit/pull/2906?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/tskit-dev/tskit/pull/2906?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev). Last update [04e04aa...2caf278](https://app.codecov.io/gh/tskit-dev/tskit/pull/2906?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tskit-dev).
lkirk commented 5 months ago

re: docstrings, there's already one present in the code for r2, so I left that one in place but got rid of all of the ones in this PR.

Input validation testing is done, all testable branches are covered.

I'll open an issue for further NaN discussions.

lkirk commented 5 months ago

I'm a little confused by the results of the coverage suite. It's annotating lines on the diff, but the patch coverage is reported as 100%.

I've opened #2907 to further investigate the NaN behavior.

Squashed down preemptively, but let me know if there's anything else needed here.