-
Notifications
You must be signed in to change notification settings - Fork 77
Remove tsk_diff_iter_t #3221
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
Remove tsk_diff_iter_t #3221
Conversation
550cbb3
to
be8a0b2
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3221 +/- ##
==========================================
- Coverage 89.63% 89.61% -0.03%
==========================================
Files 28 28
Lines 32004 31893 -111
Branches 5879 5870 -9
==========================================
- Hits 28688 28581 -107
+ Misses 1886 1884 -2
+ Partials 1430 1428 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
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 don't think we ever documented this, so I'm happy with ditching it.
@molpopgen is the only person I can think of that this would potentially affect. Are you happy for us to ditch this? The functionality is replaced by the much more efficient and flexible tree_iter stuff.
@molpopgen Said it was ok at #2797 (comment) |
It is fine for removal -- but even w/the tree iter stuff, isn't it often much more efficient to use edge differences for doing various calculations? (That has always been my experience.) Also -- what tree iter stuff? (Sorry if that question seems odd, but I find a lot of the changes on the C side to be a bit opaque/undescribed.) |
It's based on the "tree position" class that we've been working towards as a way of encapsulating the state of along the genome traversals in a better way. Some pointers:
This is also the basis of the Numba interface we'll be adding (#3225 ) |
Fixes #2797