-
Notifications
You must be signed in to change notification settings - Fork 79
Document migrations as a legacy feature, and tidy those docs #3348
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3348 +/- ##
=======================================
Coverage 89.76% 89.76%
=======================================
Files 29 29
Lines 31292 31292
Branches 5738 5738
=======================================
Hits 28089 28089
Misses 1794 1794
Partials 1409 1409
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
jeromekelleher
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you check what msprime actually does here, please?
|
Honestly I'm not sure it's worth clarifying this - it's basically a very rarely used feature of msprime which we wouldn't bother putting in now (having been made redundant by tracking migration nodes). |
|
I would vote for just saying this is a legacy thing that was tied to an old version of msprime, which may be removed from the data model entirely at some point. We do nothing at all with these in tskit. |
|
I would be +1 for removing it. But I think if it is in there we should probably clarify what Should we put a marker into the table definition to say that this is something that could be removed from the tskit data model (which would solve a number of other issues, I think, such as migrations not being supported in various methods, most notably |
I think so yeah. We can also just put in a |
8cf49c6 to
b3af51e
Compare
|
Great points about marking-as-legacy and also linking to the extensive section(s) in the msprime pages, which have much more info. I have done both of these in the latest commit, and corrected my source/dest confusion (with a link to the definition of time section in the msprime docs). I'm really very happy with marking this as an part of the data model that could be removed, and I think it's good to give people warning of that. |
b3af51e to
90bdafb
Compare
|
So to be clear, I regard the |
a52129b to
595fa50
Compare
jeromekelleher
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo
6f0dbeb to
8de2994
Compare
Also fixes #1157
As per discussion below: