-
Notifications
You must be signed in to change notification settings - Fork 79
Take a copy when making arrays for asdict #1029
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 #1029 +/- ##
=======================================
Coverage 93.69% 93.69%
=======================================
Files 26 26
Lines 20846 20848 +2
Branches 859 859
=======================================
+ Hits 19531 19533 +2
Misses 1277 1277
Partials 38 38
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
55cb126 to
71c5ba5
Compare
|
📖 Docs for this PR can be previewed here |
74f5be3 to
be2c27f
Compare
|
I've also modified the tests to use a fixture, this reduces the test time to 25% of what it was. This helps in the stress test which I have run to 2000 iterations without seeing any leaks. |
jeromekelleher
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!
| lwt = lwt_module.LightweightTableCollection() | ||
| lwt_dict = lwt.asdict() | ||
| del lwt | ||
| print(lwt_dict) |
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.
Stray print
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.
Dammit! So close!
be2c27f to
8b464a4
Compare
Description
The
tskit.LightweightTableCollectionandtskit.TableCollectionwere not taking copies of arrays, leading to lifecycle errors where the array could be deallocated.Fixes #1025
PR Checklist: