Skip to content

Allow Demography init from provenance with preset pop IDs #2370

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hyanwong
Copy link
Member

@hyanwong hyanwong commented May 23, 2025

The last step to allow demographies to be instantiated from provenance entries. We exceptionally allow a demography.Population object to be created with an existing ID, but only if generated when decoding provenance JSON. This ensures that the IDs in the events etc match with what we expect. It's a slight hack in that we check for the error message. The alternative is to pick a different error (?RuntimeError) or make our own bespoke error that can be allowed through in this case.

@hyanwong hyanwong force-pushed the pop-id-provenance-use branch from 7403228 to 28bf96b Compare May 23, 2025 18:30
Copy link

codecov bot commented May 23, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.85%. Comparing base (8d65577) to head (28bf96b).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2370   +/-   ##
=======================================
  Coverage   90.85%   90.85%           
=======================================
  Files          20       20           
  Lines       11993    12002    +9     
  Branches     2330     2332    +2     
=======================================
+ Hits        10896    10905    +9     
  Misses        606      606           
  Partials      491      491           
Flag Coverage Δ
C 90.85% <100.00%> (+<0.01%) ⬆️
python 98.69% <100.00%> (+<0.01%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@hyanwong
Copy link
Member Author

hyanwong commented May 23, 2025

With this in place, you can plot a demes demography from a tree seq generated by stdpopsim, without having to install stedpopsim, know the model etc., which is neat:

import demesdraw
ts = tskit.load("from_msprime_stdpopsim.trees")

for provenance in ts.provenances():
    try:
        cmd, params = msprime.provenance.parse_provenance(provenance, ts)
        if cmd == "sim_ancestry":
            demesdraw.tubes(params["demography"].to_demes())
    except ValueError
        pass

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.

I think we need a different way of instantiating Demography objects

try:
demography_object.__init__(**obj)
except ValueError as e:
if str(e).startswith("Population ID should not be set"):
Copy link
Member

Choose a reason for hiding this comment

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

This control flow by exception logic is fragile and distributes the logic of creating Demography objects around different modules. We should add a Demography.fromdict method that does this, and can be tested independently;.

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.

2 participants