Skip to content

Conversation

@benjeffery
Copy link
Member

@benjeffery benjeffery commented Nov 13, 2025

Fixes #3323

@benjeffery benjeffery marked this pull request as ready for review November 13, 2025 17:25
@benjeffery
Copy link
Member Author

@jeromekelleher Annoyingly we have to do some extra book-keeping here for duplicate nodes. Would appreciate some thoughts on the approach.

@codecov
Copy link

codecov bot commented Nov 13, 2025

Codecov Report

❌ Patch coverage is 63.63636% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.78%. Comparing base (488b02b) to head (9b82ae7).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
c/tskit/genotypes.c 60.00% 5 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3325      +/-   ##
==========================================
- Coverage   89.80%   89.78%   -0.02%     
==========================================
  Files          29       29              
  Lines       31062    31075      +13     
  Branches     5687     5690       +3     
==========================================
+ Hits        27894    27902       +8     
- Misses       1779     1783       +4     
- Partials     1389     1390       +1     
Flag Coverage Δ
c-tests 86.82% <60.00%> (-0.03%) ⬇️
lwt-tests 80.38% <ø> (ø)
python-c-tests 87.05% <ø> (ø)
python-tests 98.84% <100.00%> (+<0.01%) ⬆️
python-tests-no-jit 33.54% <0.00%> (-0.01%) ⬇️
python-tests-numpy1 50.09% <0.00%> (-0.02%) ⬇️

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

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

@jeromekelleher
Copy link
Member

I don't think it's worth doing, no. I tried before and it's too complicated with not that big a gain in flexibility. We've basically painted ourselves into a corner here I think and should just live with it.

@benjeffery
Copy link
Member Author

One option is to allow duplicate nodes at the alignments level by just copying there after decoding.

@jeromekelleher
Copy link
Member

I vote we drop it and live with slightly annoying workarounds. It's not a backwards compatible thing, can easily be changed post 1.0

@benjeffery benjeffery closed this Nov 19, 2025
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.

Allow duplicate nodes in tsk_variant_init

2 participants