Skip to content

Conversation

@benjeffery
Copy link
Member

@benjeffery benjeffery commented Nov 10, 2025

STACKED ON #3313

Fixes #1896

I've done this purely in Python so we can agree on the semantics and correctness before implementing in C.

@benjeffery benjeffery changed the title Alignments2 Missingness support in alignments Nov 10, 2025
@codecov
Copy link

codecov bot commented Nov 10, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.80%. Comparing base (8e6b1e6) to head (7f0afbb).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3317   +/-   ##
=======================================
  Coverage   89.79%   89.80%           
=======================================
  Files          29       29           
  Lines       31056    31062    +6     
  Branches     5685     5687    +2     
=======================================
+ Hits        27888    27894    +6     
  Misses       1779     1779           
  Partials     1389     1389           
Flag Coverage Δ
c-tests 86.84% <ø> (ø)
lwt-tests 80.38% <ø> (ø)
python-c-tests 87.05% <ø> (ø)
python-tests 98.84% <100.00%> (+<0.01%) ⬆️
python-tests-no-jit 33.55% <0.00%> (-0.03%) ⬇️
python-tests-numpy1 50.10% <0.00%> (-0.04%) ⬇️

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

Files with missing lines Coverage Δ
python/tskit/text_formats.py 100.00% <ø> (ø)
python/tskit/trees.py 98.89% <100.00%> (+<0.01%) ⬆️
python/tskit/util.py 99.28% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@benjeffery benjeffery added this to the Python 1.0 milestone Nov 10, 2025
@jeromekelleher
Copy link
Member

This looks good - it's reassuring that there was so many tests that just needed to be enabled and seemed to work. Presumably I must have thought the details through when slogging through this the last time.

@benjeffery benjeffery force-pushed the alignments2 branch 2 times, most recently from be8dec4 to fe7724c Compare November 11, 2025 16:12
@benjeffery benjeffery marked this pull request as ready for review November 11, 2025 16:12
@benjeffery benjeffery mentioned this pull request Nov 11, 2025
@benjeffery
Copy link
Member Author

@jeromekelleher I've modified alignments so that the supplied reference sequence has to be the same length as the tree sequence - note that this is a breaking change to the previous behaviour.

I've filed #3318 and #3319 as follow ups.

Copy link
Member

@jeromekelleher jeromekelleher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sgtm. Let's update the changleog and merge? Subsequent updates are just perf, right?

@benjeffery
Copy link
Member Author

Changlog added

Copy link
Member

@jeromekelleher jeromekelleher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Spotted a typo

**Breaking Changes**

- The ``reference_sequence`` argument to ``TreeSequence.alignments`` is now required
to be the same length as the tree sequence. Previsouly it was required to be the length
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
to be the same length as the tree sequence. Previsouly it was required to be the length
to be the same length as the tree sequence. Previously it was required to be the length

@benjeffery benjeffery enabled auto-merge November 12, 2025 13:25
@benjeffery benjeffery added this pull request to the merge queue Nov 12, 2025
Merged via the queue into tskit-dev:main with commit 488b02b Nov 12, 2025
18 checks passed
@benjeffery benjeffery deleted the alignments2 branch November 12, 2025 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support missing data in alignments

2 participants