tskit
tskit copied to clipboard
tskit-dev
Remove tsk_diff_iter_t
After #2786 we don't actually use tsk_diff_iter_t at all in the library. As it's not part of the public API (and it causes some annoying problems internally, e.g. here) I would vote for removing it.
The only reason not to as far as I can see is that the Rust API currently uses it. The best outcome I think would be for the Rust API to start using the tsk_tree_position_t class as a replacement, which is probably ok?
The main problem with this I think is that the tsk_tree_position_t class only supports working from a tsk_treeseq_t, not a tsk_table_collection_t. The main reason for this is that it's useful to have the breakpoints array around. While we can do without the breakpoints for the simple prev and next operations, it would be much more awkward to do the general seek operations without it. So, I'd be inclined to keep things simple, and just have the tsk_tree_position_t depend on the tree sequence rather than table collection, but I guess we can imagine a way of just supported in next and prev if we don't have the breakpoints array.
Of course we can also just keep the tsk_diff_iter_t class.
What do you think @molpopgen?
I'd have to think a bit, but it is probably okay. Rust hides the existing API behind a "streaming" (non-owning) iterator abstraction. So dropping the low-level type breaks semver (even though it is undocumented, rust is exposing the C API for convenience and the loss of a type will get picked up as a breakage by CI) but will not break the use of the edge diff iterator once its internals are updated.
The only reason not to as far as I can see is that the Rust API currently uses it. The best outcome I think would be for the Rust API to start using the
tsk_tree_position_tclass as a replacement, which is probably ok?
I do have specific reasons for wanting to do edge diffs on a table collection. I haven't had time to explore, but I think that there are some use cases for forward simulation.
Fortunately, tskit-rust does not currently provide an edge difference iterator from tables. IIRC, that's because there's been no C API release with those changes?
Upon further reflection: if we remove this type and I do need it, it is trivial-ish to re-implement later.