Individual parents merge

[ivan-krukov]

Description

Adding a ragged parents column to the individuals table.

Fixes #852

This is a clean version of the #866 PR

PR Checklist:

• Tests that fully cover new/changed functionality.
• Documentation including tutorial content if appropriate.
• Changelogs, if there are API changes.

[benjeffery]

Nice one on getting this rebased, hope it wasn't too much trouble! I'll have a detailed look tomorrow.

[codecov]

Codecov Report

Merging #1125 (8ebfd8b) into main (8370ca7) will decrease coverage by 0.06%.
The diff coverage is 86.30%.

@@            Coverage Diff             @@
##             main    #1125      +/-   ##
==========================================
- Coverage   93.71%   93.65%   -0.07%     
==========================================
  Files          26       26              
  Lines       21082    21236     +154     
  Branches      900      900              
==========================================
+ Hits        19757    19888     +131     
- Misses       1288     1311      +23     
  Partials       37       37              

Flag	Coverage Δ
c-tests	92.36% <75.26%> (-0.16%)	⬇️
lwt-tests	92.78% <92.00%> (-0.03%)	⬇️
python-c-tests	94.89% <100.00%> (+0.03%)	⬆️
python-tests	98.64% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
c/tskit/tables.c	90.50% <75.26%> (-0.27%)	⬇️
python/_tskitmodule.c	91.37% <100.00%> (+0.06%)	⬆️
python/lwt_interface/tskit_lwt_interface.h	95.04% <100.00%> (+0.12%)	⬆️
python/tskit/tables.py	99.60% <100.00%> (+<0.01%)	⬆️
python/tskit/trees.py	97.47% <100.00%> (+<0.01%)	⬆️

---

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 8370ca7...8ebfd8b. Read the comment docs.

[ivan-krukov]

The final rebase was pretty simple - the patching process took care of everything.
Thank you for writing the detailed how-to git guides - they are a treasure.

But it looks like the same test coverage check is failing this time.

There is also an outstanding issue of documenting these changes. Does this need to happen before the merge?

[jeromekelleher]

LGTM @ivan-krukov! I've gone through fairly carefully, and I don't see any show-stoppers.

There's some collatoral damage here from the merge where you've clobbered some recent metadata changes. You'll need to drop these from the diff (@benjeffery, any thoughts on the easiest way to do this?)

One thing we'll need to do as a follow up is to update simplify to keep track of the individual ID references in parents so that the IDs are correct afterwards.

Other than that, I think we just need to go through the standard steps for bumping file format version numbers and so on. These should all be done as follow ups, though, I think it's best to get these basic changes in soon so we don't have to keep rebasing them.

[jeromekelleher]

LGTM @ivan-krukov! One tiny nit left I think, otherwise good to squash and merge. The squash should go cleanly, now.

@benjeffery, can you open up some issues to track the remaining things that need to be addressed before we release please?

[benjeffery]

Great stuff, I've been through closely and logged issues for follow up work.

[benjeffery]

@ivan-krukov As @jeromekelleher said one nit about a comment that isn't consistent, then a squash and we can merge.
I recently added columns to the data model so I know how much work this is. Nice one!

[jeromekelleher]

@ivan-krukov, can we get this updated please? There's a significant chance of this PR getting out of date again if we don't merge it ASAP!

[benjeffery]

I've just force-pushed a fix and squash to this PR. Will merge if all green.

[jeromekelleher]

Segfault on some C tests - are you looking at this @benjeffery, or should @ivan-krukov pick it up?

[benjeffery]

I'll check it out.

[benjeffery]

The segfault was in the text parsing code for one of the example trees where the individuals string did not have parents (test_simplest_unary_with_individuals). I've fixed by adding -1,-1 parents to that example.

[ivan-krukov]

I am so sorry that I am late for this. Thank you for resolving.

[jeromekelleher]

Woohoo, great to see this merged! Thanks for all the hard work @ivan-krukov, adding new columns is always a marathon. (Thanks too @benjeffery!)

[benjeffery]

I am so sorry that I am late for this. Thank you for resolving.

No worries @ivan-krukov, it was a very small and quick fix. Thanks for the PR!