Add error for taking table with no edge metadata #2233
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2233 +/- ##
=======================================
Coverage 93.30% 93.30%
=======================================
Files 27 27
Lines 26003 26009 +6
Branches 1165 1165
=======================================
+ Hits 24262 24268 +6
Misses 1711 1711
Partials 30 30
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
jeromekelleher
left a comment
jeromekelleher
left a comment
There was a problem hiding this comment.
Good catch, but I think the implementation is uncovering another bug?
c/tskit/trees.c
Outdated
| tsk_memset(self, 0, sizeof(*self)); | ||
| if (options & TSK_TAKE_OWNERSHIP) { | ||
| self->tables = tables; | ||
| if (tables->edges.options & TSK_NO_EDGE_METADATA) { |
There was a problem hiding this comment.
Shouldn't the flag be TSK_TABLE_NO_METADATA though? I thought that was the point of that flag - any table that has no metadata would use it. the TSK_TC_NO_EDGE_METADATA was a way to send that flag to edge_table_init.
Maybe we should wait until #2224 is in before tackling this.
There was a problem hiding this comment.
Ah, they have the same value as I haven't rebased, so if I had rebased my mistake would have been revealed.
I've rebased and fixed this.
benjeffery
force-pushed
the
test_take_no_edge_metadata
branch
from
20d3c63 to
3f95078
Compare
|
@jeromekelleher I think this is ready to go now. |
2 participants
Fixes #2207
Added an error for this case.