Refseq update

[jeromekelleher]

WIP, still working out the details.

[codecov]

Codecov Report

Merging #1944 (de6cd9d) into main (4f09c03) will increase coverage by 0.02%.
The diff coverage is 93.00%.

❗ Current head de6cd9d differs from pull request most recent head 43921cc. Consider uploading reports for the commit 43921cc to get more accurate results

@@            Coverage Diff             @@
##             main    #1944      +/-   ##
==========================================
+ Coverage   93.32%   93.35%   +0.02%     
==========================================
  Files          27       27              
  Lines       25207    25494     +287     
  Branches     1108     1108              
==========================================
+ Hits        23525    23800     +275     
- Misses       1648     1660      +12     
  Partials       34       34              

Flag	Coverage Δ
c-tests	92.37% <96.22%> (+0.04%)	⬆️
lwt-tests	89.05% <88.34%> (-0.10%)	⬇️
python-c-tests	72.07% <90.95%> (+0.38%)	⬆️
python-tests	98.80% <100.00%> (+<0.01%)	⬆️

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

Impacted Files	Coverage Δ
python/lwt_interface/tskit_lwt_interface.h	90.81% <88.34%> (-0.23%)	⬇️
python/_tskitmodule.c	90.95% <95.65%> (+0.13%)	⬆️
c/tskit/tables.c	90.26% <96.07%> (+0.07%)	⬆️
c/tskit/trees.c	94.97% <100.00%> (+<0.01%)	⬆️
python/tskit/metadata.py	98.99% <100.00%> (+0.06%)	⬆️
python/tskit/tables.py	98.94% <100.00%> (+<0.01%)	⬆️
python/tskit/trees.py	97.80% <100.00%> (+<0.01%)	⬆️
... and 2 more

---

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 4f09c03...43921cc. Read the comment docs.

[jeromekelleher]

I think this is ready for a look @benjeffery, if you wouldn't mind. I think most of the stuff has been covered, other than some issues that have popped up on the TableCollection tests, some extra file format tests, and some minimal docs.

Do we bump the file format and dict encoding minor numbers for this?

[jeromekelleher]

Good to get your eyes on this if possible please @benjeffery - hopefully I can get it finished in one more pass, it would be really helpful to see what you think.

[benjeffery]

Good to get your eyes on this if possible please @benjeffery - hopefully I can get it finished in one more pass, it would be really helpful to see what you think.

Sorry, having a proper look now.

[jeromekelleher]

OK, this is ready for review I think! I think I've covered all the high-level things and opened issues for the stuff we haven't done. Do you see anything missing @benjeffery?

[benjeffery]

Couple of small things, overall I think this is great.
I have some concern about .data being a string at this point when we know we want to do zero-copy via arrays. It would likely be a breaking change to do so. Maybe we use as_string or similar for now at the public API? Then data can be the array later?

[jeromekelleher]

have some concern about .data being a string at this point when we know we want to do zero-copy via arrays. It would likely be a breaking change to do so. Maybe we use as_string or similar for now at the public API? Then data can be the array later?

I've opened issues #1989 and #1988 to address this - what do you think? Basically, the .data attribute is there, but high-level users won't really be seeing it.

[jeromekelleher]

Thanks for the review @benjeffery, I think I've addressed those points. Ready to go I think!

[benjeffery]

have some concern about .data being a string at this point when we know we want to do zero-copy via arrays. It would likely be a breaking change to do so. Maybe we use as_string or similar for now at the public API? Then data can be the array later?

I've opened issues #1989 and #1988 to address this - what do you think? Basically, the .data attribute is there, but high-level users won't really be seeing it.

Great, if data isn't the way that high-level users get at the sequence then it's all good.