-
Notifications
You must be signed in to change notification settings - Fork 78
Delete_older operation #2302
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
Delete_older operation #2302
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2302 +/- ##
==========================================
- Coverage 93.29% 93.23% -0.06%
==========================================
Files 28 28
Lines 26482 26565 +83
Branches 1211 1211
==========================================
+ Hits 24706 24769 +63
- Misses 1744 1764 +20
Partials 32 32
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
The name seems good! We also have |
|
The definition also seems good - it does what we want for decapitate. The docstring will definitely need a "seealso decapitate" note, as people might grab this when they need that, though. |
e2206e2 to
7d8fda4
Compare
a0080c2 to
9465099
Compare
|
I think this is ready to go - I'll update the changelog and docstring (to say maybe you want decapitate?) for this when decapitate is done in #2240 |
| tsk_table_collection_t *self, tsk_flags_t options); | ||
| int tsk_table_collection_compute_mutation_times( | ||
| tsk_table_collection_t *self, double *random, tsk_flags_t TSK_UNUSED(options)); | ||
| tsk_table_collection_t *self, double *random, tsk_flags_t options); |
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.
whoops - you've un-marked tsk_table_collection_compute_mutation_times's options as unused (they are actually unused; I just checked)
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.
| tsk_table_collection_t *self, double *random, tsk_flags_t options); | |
| tsk_table_collection_t *self, double *random, tsk_flags_t TSK_UNUSED(options)); |
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.
You don't need to do this in the header file though, it's just in the .c.
petrelharp
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.
LGTM!
b5b7f1c to
51445db
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.
LGTM.
51445db to
be18b7e
Compare
Next step on the road to decapitate (#2240, #2236). The idea is to implement this fully as a TableCollection method (not bothering with ts equivalent) in C.
Then decapitate is just the composition of this function and
split_edges. The advantage of doing this function on its own, is that we then don't have to do a C version of decapitate (which requires faddling around with all the node default args again).What do you think of the definition @petrelharp? Is the name ok? I haven't tested the mutation and migration stuff so far, could well be wrong.