-
Notifications
You must be signed in to change notification settings - Fork 72
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
Added methods to sort and squash segments in the IbdFinder output. #2460
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2460 +/- ##
=======================================
Coverage 89.72% 89.72%
=======================================
Files 29 29
Lines 31578 31578
Branches 6117 6117
=======================================
Hits 28333 28333
Misses 1853 1853
Partials 1392 1392
Flags with carried forward coverage won't be shown. Click here to find out more. |
Hey @hyanwong! Unfortunately, we both made some pretty involved changes to the test_highlevel.py file in the past few days. I think I've fixed up the merge conflict, but I can't figure out how to get my tests to run with the argument that I added into |
It was @benjeffery who fixed up |
I'll take a look now |
I think the simplest thing here is to remove |
Thanks @benjeffery and sorry for the late reply -- I had to take a few days off to deal with some urgent moving-to-the-US things.
I added this argument because some of my changes in this PR make IbdFinder run much more slowly, to the point where they look like they're hanging. (This is likely a Python-specific problem -- as we discuss in #2459, the C code works differently because it uses an AVL tree to do the sorting). But I didn't want to omit the tests entirely, as they cover some useful edge cases, so I wanted to run them with smaller sample sizes than the ones hard-coded in. |
We can leave the slow examples in the examples - but skip them in these tests? |
Made some small changes to ibd.py to ensure consistent behaviour in tests
49ce776
to
ce924de
Compare
I've rebased this to main - the tests here only add a few seconds on my machine, will see home CI behaves. @jeromekelleher I think this is pretty complete if you want to have a quick look to check? |
ce924de
to
62115b3
Compare
@Mergifyio update |
✅ Branch has been successfully updated |
Fixes #2459 -- see the issue for discussion about why this is needed. Draft only at the moment.
PR Checklist: