Add __setitem__ to tables. #1600
Conversation
benjeffery
force-pushed
the
replace-row-2
branch
3 times, most recently
from
c2d1ffc to
d33c0ec
Compare
Codecov Report
@@ Coverage Diff @@
## main #1600 +/- ##
=======================================
Coverage 93.67% 93.67%
=======================================
Files 27 27
Lines 23276 23287 +11
Branches 1079 1084 +5
=======================================
+ Hits 21804 21815 +11
Misses 1438 1438
Partials 34 34
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
Missing some coverage here, will fix. |
benjeffery
force-pushed
the
replace-row-2
branch
from
d33c0ec to
a4ad45a
Compare
python/tskit/tables.py
Outdated
| row-like object. Metadata, will be validated and encoded according to the table's | ||
| :attr:`metadata_schema<tskit.IndividualTable.metadata_schema>`. | ||
|
|
||
| :param index: the zero-index of the row to change |
There was a problem hiding this comment.
Maybe delete the zero here? All indexes are zero-based, might just be confusing.
There was a problem hiding this comment.
This was cut and pasted from __getitem__ so I've removed it there also.
| index += len(self) | ||
| if index < 0 or index >= len(self): | ||
| raise IndexError("Index out of bounds") | ||
| else: |
There was a problem hiding this comment.
Let's just think ahead here to make sure we're not painting ourselves into a corner. What would the semantics be for other types of updates? I guess we could support other types of assignment here.
If the index argument is a slice/boolean array, then the new_row argument should be a Sequence of row_like values (of the same length as the index argument), and then we sequentially call update_row on these? Wouldn't be particularly efficient, but we could push the implementation down into C if we wanted to, and it would allow us to do things like
tables.edges[:] = tables.edges[::-1]which I'm sure would be handy.
I'm not suggesting we do this now, just making sure we leave the door open for it in the future. I think TypeError on anything that's not an Integral for the index value does this?
There was a problem hiding this comment.
Yep, that all makes sense. The code as-is raises TypeError on anything non-Integral.
python/tskit/tables.py
Outdated
| if "_offset" not in column | ||
| } | ||
|
|
||
| # Encode the meatdata - note that if this becomes a perf bottleneck it is |
There was a problem hiding this comment.
Must have been hungry when writing this, fixed.
benjeffery
force-pushed
the
replace-row-2
branch
from
a4ad45a to
1e769b8
Compare
Fixes #1545
Stacked on #1597