Skip to content

Conversation

@jeromekelleher
Copy link
Member

Closes #2287

@jeromekelleher jeromekelleher requested a review from benjeffery May 20, 2022 19:38
@jeromekelleher jeromekelleher marked this pull request as ready for review May 20, 2022 19:40
@jeromekelleher jeromekelleher force-pushed the remove-copy-in-simplify branch from af6b862 to afed65a Compare May 20, 2022 19:40
@codecov
Copy link

codecov bot commented May 20, 2022

Codecov Report

Merging #2288 (afed65a) into main (7740baa) will decrease coverage by 0.00%.
The diff coverage is 81.81%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2288      +/-   ##
==========================================
- Coverage   93.33%   93.33%   -0.01%     
==========================================
  Files          27       27              
  Lines       26134    26141       +7     
  Branches     1175     1175              
==========================================
+ Hits        24393    24398       +5     
- Misses       1711     1713       +2     
  Partials       30       30              
Flag Coverage Δ
c-tests 92.30% <81.81%> (-0.02%) ⬇️
lwt-tests 89.05% <ø> (ø)
python-c-tests 71.88% <ø> (ø)
python-tests 98.88% <ø> (ø)

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

Impacted Files Coverage Δ
c/tskit/trees.c 95.23% <81.81%> (-0.06%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7740baa...afed65a. Read the comment docs.

Copy link
Member

@benjeffery benjeffery left a comment

Choose a reason for hiding this comment

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

LGTM! Great!

@mergify mergify bot merged commit 5cce659 into tskit-dev:main May 20, 2022
@petrelharp
Copy link
Contributor

Whoah, this is very nice. This should help SLiM, @bhaller! (once I merge the final tskit C_1.0 in).

@bhaller
Copy link

bhaller commented May 21, 2022

Whoah, this is very nice. This should help SLiM, @bhaller! (once I merge the final tskit C_1.0 in).

Nice! I wasn't aware this was happening, looks helpful indeed.

@jeromekelleher
Copy link
Member Author

This should help SLiM, @bhaller! (once I merge the final tskit C_1.0 in).

I'd be surprised if it makes any difference for SLiM, since I'd imagine you guys are calling tsk_table_collection_simplify rather than tsk_treeseq_simplify.

@jeromekelleher jeromekelleher deleted the remove-copy-in-simplify branch May 21, 2022 10:26
@bhaller
Copy link

bhaller commented May 21, 2022

This should help SLiM, @bhaller! (once I merge the final tskit C_1.0 in).

I'd be surprised if it makes any difference for SLiM, since I'd imagine you guys are calling tsk_table_collection_simplify rather than tsk_treeseq_simplify.

Ah! That is true, we are.

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.

Add TSK_TAKE_OWNERSHIP in tsk_treeseq_simplify

4 participants