Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add mask to sgkit SampleData #791

Merged
merged 2 commits into from
May 15, 2023
Merged

Add mask to sgkit SampleData #791

merged 2 commits into from
May 15, 2023

Conversation

benjeffery
Copy link
Member

Fixes #786

@codecov
Copy link

codecov bot commented Jan 17, 2023

Codecov Report

Merging #791 (9e7b9fd) into main (bd36e8d) will increase coverage by 0.10%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #791      +/-   ##
==========================================
+ Coverage   93.51%   93.61%   +0.10%     
==========================================
  Files          17       17              
  Lines        5874     5905      +31     
  Branches     1051     1063      +12     
==========================================
+ Hits         5493     5528      +35     
+ Misses        250      246       -4     
  Partials      131      131              
Flag Coverage Δ
C 93.61% <100.00%> (+0.10%) ⬆️
python 96.42% <100.00%> (+0.12%) ⬆️

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

Impacted Files Coverage Δ
tsinfer/formats.py 97.80% <100.00%> (+0.30%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@jeromekelleher jeromekelleher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks nice, not particularly invasive!

tsinfer/formats.py Show resolved Hide resolved
@benjeffery
Copy link
Member Author

This needs a few more tests before merging.

@benjeffery benjeffery marked this pull request as ready for review January 24, 2023 13:57
@benjeffery benjeffery marked this pull request as draft January 25, 2023 16:01
@benjeffery
Copy link
Member Author

Converted back to draft as I'm not 100% sure the haplotype iterator is using the mask - not sure how the tests are passing if it is, but need to check.

@benjeffery benjeffery force-pushed the mask branch 2 times, most recently from 43a545c to 4cb9fd4 Compare May 5, 2023 14:09
@benjeffery benjeffery marked this pull request as ready for review May 5, 2023 14:09
@benjeffery
Copy link
Member Author

Added all the other arrays that the existing sample data used, including fallbacks for when they are missing.
Everything that isn't made by sgkit's vcf_to_zarr is optional.

Copy link
Member

@jeromekelleher jeromekelleher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - looks like we're missing coverage on one error condition, good to catch that before merge

@mergify mergify bot merged commit 6150305 into tskit-dev:main May 15, 2023
13 checks passed
@benjeffery benjeffery deleted the mask branch September 21, 2023 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sgkit: Support call_genotype_mask?
2 participants