-
Notifications
You must be signed in to change notification settings - Fork 78
Add site ID to VCF output #2107
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
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2107 +/- ##
=======================================
Coverage 93.37% 93.37%
=======================================
Files 27 27
Lines 25586 25587 +1
Branches 1159 1159
=======================================
+ Hits 23890 23891 +1
Misses 1666 1666
Partials 30 30
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
Hi, |
|
Thanks for the PR @roohy! Will review today. |
benjeffery
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! Thanks @roohy!
I've made one small suggestion to simplify the testing.
Please go ahead and add a changelog and a note in the docs.
|
Changelog and docs added. |
Great, thanks. All that is left is to fix the failing test ( |
|
Thank you! I tried doing them so as not to take any more of your time. Please let me know if I am still forgetting something. |
92e7125 to
fb29186
Compare
benjeffery
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've squashed and rebased.
Thanks for this @roohy, great to have you on the contributors list.
|
Thanks @benjeffery for your help. I hope I can make more/bigger contributions to your great software in the future! |
Description
I added modified to python/tskit/vcf.py file to write the site ID of the variant in the ID field of the VCF file output.
I added a test to check VCF variant ID and site ID in the ts object are the same in test_vcf.py file under the TestInterface class just in case. I did not add anything to documentation or the changelogs since I first wanted to submit this draft pull request and ask first since the change is very minimal.
Fixes #2103
PR Checklist: