add delete_sites table function

[hyanwong]

Fixes #363

[hyanwong]

Do we also want this as a tree sequence function?

I.e. do we want to be able to do ts_new = ts.delete_sites([1,2,3]) as well as ts.dump_tables().delete_sites([1,2,3])?

[codecov]

Codecov Report

Merging #364 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #364      +/-   ##
==========================================
+ Coverage    86.5%   86.52%   +0.01%     
==========================================
  Files          20       20              
  Lines       14049    14069      +20     
  Branches     2747     2749       +2     
==========================================
+ Hits        12153    12173      +20     
  Misses        977      977              
  Partials      919      919

Flag	Coverage Δ
#c_tests	87.61% <100%> (+0.02%)	⬆️
#python_c_tests	90.38% <100%> (+0.02%)	⬆️
#python_tests	99.25% <100%> (ø)	⬆️

Impacted Files	Coverage Δ
python/tskit/tables.py	99.79% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0488ddd...621ea8f. Read the comment docs.

[jeromekelleher]

I've updated this to simplify the interface (it's just adding complexity for no real gain supporting boolean selector arrays) and to add some tests. The existing tests were weak.

There's a bug to be fixed when we have multiple mutations.

[hyanwong]

I've updated this to simplify the interface (it's just adding complexity for no real gain supporting boolean selector arrays)

Right

There's a bug to be fixed when we have multiple mutations.

Do you want me to do this?

[jeromekelleher]

Do you want me to do this?

Yes, if you want the PR merged.

[hyanwong]

This should pass once #366 or equivalent is merged

[jeromekelleher]

Good fix, can you rebase please.

[hyanwong]

Rebased. I think the fact that I got the parent mutations mixed up (I assumed they contained node IDs, not mutation IDs) is a good argument for merging this, rather than relying on people to cook up their own, potentially buggy, equivalent.

[jeromekelleher]

OK, I think this is ready to merge. I've added some clarifications on the ordering requirements for IDs (there are none) and what happens when we have duplicate IDs.

[petrelharp]

This is great! We will have use for this. :yay:

[hyanwong]

@petrelharp - great. I think some of the code in keep_intervals / delete_intervals might be simplified by calling this function too, if we want to reduce code duplication.