Add Demography.to_demes().#1724
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1724 +/- ##
==========================================
+ Coverage 91.14% 91.38% +0.24%
==========================================
Files 20 20
Lines 10271 10393 +122
Branches 2159 2200 +41
==========================================
+ Hits 9361 9498 +137
+ Misses 468 454 -14
+ Partials 442 441 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
jeromekelleher
left a comment
There was a problem hiding this comment.
Looks good - is it worth letting the DemographyDebugger do some of the heavy lifting though? It's a good way of avoiding some of the multiple implementations of the event logic.
|
I updated to use the demography debugger for the migration matrices, and to get the deme start/end times with respect to the population's ACTIVE state. It's also desirable to check start/end times of demes based on mass migrations with proportion 1, to support legacy models, so I've left that in. Also, the big loop still iterates the events rather than the demography debugger epochs --- there's some quirk with the granularity of times returned by the demography debugger epochs such that the round tripping gives problems. I don't think it's worth the effort to try and figure that out just to shuffle a couple of lines of code. |
|
Oh, and I updated the docstring to mention the bottleneck events can't be converted, and that legacy models could be misspecified and in general may not be convertible. |
jeromekelleher
left a comment
There was a problem hiding this comment.
Looks great, thanks @grahamgower. One tiny typo spotted.
I'm generally a bit uneasy about how much logic is going in here, but I guess it can't be avoided unless we somehow modified the low-level C code to emit a demes model through the debug interface.
|
Typo fixed, rebased, and lightly squashed. |
|
Ah, sorry - can you update the CHANGELOG with this please? This is a big feature, don't want to forget about it on release. |
|
Changelog updated. |
Closes #1721.
WIP, but seems to be behaving itself. At least, conversion of the
Demography._*_model()models look correct when plotting them with demesdraw.