Merge pull request #225 from grahamgower/start_time

Fix Graph.in_generations() to convert the deme start_time.

CHANGELOG.rst

@@ -1,3 +1,16 @@
+********************
+0.1.0a3 - 2021-02-25
+********************
+
+**Bug fixes**:
+
+- Fix ``Graph.in_generations()`` to also convert the ``Deme.start_time`` field.
+  Thanks to :user:`apragsdale` for reporting the problem.
+  (:user:`grahamgower`, :issue:`224`, :pr:`225`).
+- Fix ``assert_close()`` and ``is_close()`` equality checks to compare the deme
+  ``start_time``.
+  (:user:`grahamgower`, :issue:`224`, :pr:`225`).
+
 ********************
 0.1.0a2 - 2021-02-24
 ********************

demes/demes.py

@@ -992,6 +992,7 @@ class Deme:
 
     :ivar str id: A string identifier for the deme.
     :ivar str description: A description of the deme. May be ``None``.
+    :ivar float start_time: The time at which the deme begins to exist.
     :ivar ancestors: List of string identifiers for the deme's ancestors.
         This may be ``None``, indicating the deme has no ancestors.
     :vartype ancestors: list of str
@@ -1010,6 +1011,7 @@ class Deme:
             [attr.validators.instance_of(str), nonzero_len]
         )
     )
+    start_time: Time = attr.ib(validator=[int_or_float, positive])
     ancestors: List[ID] = attr.ib(
         validator=attr.validators.deep_iterable(
             member_validator=attr.validators.and_(
@@ -1030,7 +1032,6 @@ class Deme:
             iterable_validator=attr.validators.instance_of(list),
         )
     )
-    start_time: Time = attr.ib(validator=[int_or_float, positive])
 
     @ancestors.validator
     def _check_ancestors(self, _attribute, _value):
@@ -1145,6 +1146,9 @@ def assert_close(
             self.__class__ is other.__class__
         ), f"Failed as other deme is not instance of {self.__class__} type."
         assert self.id == other.id
+        assert math.isclose(
+            self.start_time, other.start_time, rel_tol=rel_tol, abs_tol=abs_tol
+        ), f"Failed for start_time {self.start_time} != {other.start_time} (other)."
         assert isclose_deme_proportions(
             self.ancestors,
             self.proportions,
@@ -1810,6 +1814,7 @@ def in_generations(self):
         if generation_time is not None:
             graph.generation_time = None
             for deme in graph.demes:
+                deme.start_time /= generation_time
                 for epoch in deme.epochs:
                     epoch.start_time /= generation_time
                     epoch.end_time /= generation_time
@@ -2024,7 +2029,6 @@ def coerce_numbers(inst, attribute, value):
         data = attr.asdict(self, filter=filt, value_serializer=coerce_numbers)
         # translate to spec data model
         for deme in data["demes"]:
-            deme["start_time"] = deme["epochs"][0]["start_time"]
             for epoch in deme["epochs"]:
                 del epoch["start_time"]
                 if epoch["selfing_rate"] == 0:

docs/conf.py

@@ -47,6 +47,7 @@
     "sphinx.ext.autodoc",
     "sphinx.ext.todo",
     "sphinx.ext.viewcode",
+    "sphinx_issues",
     "jupyter_sphinx",
 ]
 
@@ -72,3 +73,6 @@
 # relative to this directory. They are copied after the builtin static files,
 # so a file named "default.css" will overwrite the builtin "default.css".
 html_static_path = ["_static"]
+
+# Github repo, for sphinx_issues
+issues_github_path = "popsim-consortium/demes-python"

requirements.txt

@@ -11,4 +11,5 @@ pytest-cov==2.11.1
 ruamel.yaml==0.16.12
 sphinx==3.4.3
 sphinx_rtd_theme==0.5.1
+sphinx_issues==1.2.0
 stdpopsim==0.1.2

tests/test_demes.py

@@ -1858,8 +1858,10 @@ def check_in_generations(self, dg1):
         dg1_copy = copy.deepcopy(dg1)
         dg2 = dg1.in_generations()
         # in_generations() shouldn't modify the original
+        dg1_copy.assert_close(dg1)
         self.assertEqual(dg1.asdict(), dg1_copy.asdict())
         # but clearly dg2 should now differ
+        assert not dg1.isclose(dg2)
         self.assertNotEqual(dg1.asdict(), dg2.asdict())
 
         # Alternate implementation, which recurses the object hierarchy.
@@ -1887,14 +1889,19 @@ def divide_time_attrs(obj):
             divide_time_attrs(dg)
             return dg
 
+        dg2.assert_close(in_generations2(dg1))
         self.assertEqual(in_generations2(dg1).asdict(), dg2.asdict())
+
         # in_generations2() shouldn't modify the original
+        dg1.assert_close(dg1_copy)
         self.assertEqual(dg1.asdict(), dg1_copy.asdict())
 
         # in_generations() should be idempotent
         dg3 = dg2.in_generations()
+        dg2.assert_close(dg3)
         self.assertEqual(dg2.asdict(), dg3.asdict())
         dg3 = in_generations2(dg2)
+        dg2.assert_close(dg3)
         self.assertEqual(dg2.asdict(), dg3.asdict())
 
     def test_in_generations(self):