remove SLiM metadata; closes #2534 - #2535
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2535 +/- ##
=======================================
Coverage 93.83% 93.84%
=======================================
Files 20 20
Lines 12106 12060 -46
Branches 2243 2242 -1
=======================================
- Hits 11360 11318 -42
+ Misses 567 564 -3
+ Partials 179 178 -1
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.
LGTM, spotted on trivial thing.
Did we not set a metadata schema for the slim mutation metadata?
I don't think we need to wait until SLiM 6 is released to merge, but we shouldn't release msprime until it is, though.
Might be worth noting that this also means that these mutations won't work with SLiM < 6 now (this isn't obvious from the CHANGELOG).
| assert t1.sites == t2.sites | ||
| # Drop the mutation metadata - we're validating that elsewhere and | ||
| # it's not worth complicating the Python generator with it. | ||
| t1.mutations.set_columns( |
There was a problem hiding this comment.
This set_columns is redundant now, right?
Huh, yeah, I guess not. |
|
This should be ready now! |
This just rips out all metadata stuff from the
SLiMMutationModel. We should not merge this until SLiM v6 is released.Potential breakage: if someone runs
then they get a warning saying that
typeandslim_generationwill be ignored.If someone runs
or
then they get a TypeError because the only valid positional argument is the first one.
The latter is necessary to exclude because previously if someone ran this they'd get a model with type 1 and next ID 10; now (if both arguments were positional) they'd get a model with next ID 1 and block_size 10.
One possible problem: previously, the only required argument was mutation type, and it was the first argument. So, if someone ran
then they would get a model with mutation type k and next ID 0. Now, they would get a model with next ID k. This will cause no practical problems - both "mutation type" and "next ID" have to be a nonnegative integers, and so this just means msprime will not simulate mutations with SLiM ID 0, ..., k-1 when you might have thought it would.
The alternative to this PR would be to just leave everything alone: msprime puts in unneeded metadata, and we can just safely ignore it downstream. But, this is confusing.