Add TableCollection dump/load #986
Conversation
|
📖 Docs for this PR can be previewed here |
Codecov Report
@@ Coverage Diff @@
## main #986 +/- ##
==========================================
- Coverage 93.71% 93.64% -0.07%
==========================================
Files 26 26
Lines 20622 20680 +58
Branches 836 835 -1
==========================================
+ Hits 19326 19366 +40
- Misses 1259 1277 +18
Partials 37 37
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
benjeffery
force-pushed
the
tc-dump
branch
4 times, most recently
from
606db0a to
2978a9a
Compare
|
Codecov isn't happy due to lines in _tskitmodule.c which it says are not covered. When I set a breakpoint on those lines it gets hit though! Not sure what is happening there. |
jeromekelleher
left a comment
jeromekelleher
left a comment
There was a problem hiding this comment.
LGTM, but a few comments above.
python/tskit/tables.py
Outdated
| tc._ll_tables = ll_tc | ||
| return tc | ||
|
|
||
| def dump(self, file_or_path, skip_checks=False): |
There was a problem hiding this comment.
I'm not sure we want the skip_checks argument here as it might have some unwanted effects, and might be expensive. I think we should just write out the state as it is.
If we do have the checks we should probably implement at the C level using the check integrity function, rather than just calling self.tree_sequence().
|
Codecov has definitely gone a bit mad on the TableCollection_alloc coverage - that function must be being called. So, we can ignore codecov here I think. |
|
@jeromekelleher Fixed up, still needs a squash as left several commits for easy change viewing. |
jeromekelleher
left a comment
jeromekelleher
left a comment
There was a problem hiding this comment.
Looks great, thanks @benjeffery! There's one potential cleanup, but merge away whenever.
python/tests/test_lowlevel.py
Outdated
| with open(tmp_path / "temp.trees", "rb") as f: | ||
| ts2 = _tskit.TreeSequence() | ||
| ts2.load(f) | ||
| assert ts.get_num_samples() == ts2.get_num_samples() |
There was a problem hiding this comment.
I did all this stuff long-hand before because we didn't have the table collection equality operator - we could just replace with the standard ts.tables == ts2.tables now.
There was a problem hiding this comment.
As this is the low-level ts I had to go with:
tc = _tskit.TableCollection(ts.get_sequence_length())
ts.dump_tables(tc)
tc2 = _tskit.TableCollection(ts2.get_sequence_length())
ts2.dump_tables(tc2)
assert tc.equals(tc2)
|
yay! this will simplify some other code! |
4 participants
Description
Adds
dumpandloadtoTableCollection.Fixes #14
PR Checklist: