From 9b2fc1420bffa413d389c181786418d19701ba56 Mon Sep 17 00:00:00 2001
From: "Kevin R. Thornton" <krthornt@uci.edu>
Date: Tue, 1 Aug 2023 12:47:14 -0700
Subject: [PATCH] Improve recombination interval validation in _evolvets
 (#1155)

Fixes #1153
---
 doc/misc/changelog.md                     |  6 ++++
 fwdpy11/_evolvets.py                      | 31 ++++++++++++++++-----
 tests/demes-spec                          |  2 +-
 tests/test_ModelParams_with_recregions.py | 34 +++++++++++++++++------
 4 files changed, 57 insertions(+), 16 deletions(-)

diff --git a/doc/misc/changelog.md b/doc/misc/changelog.md
index c0c63a4f9..4e869d942 100644
--- a/doc/misc/changelog.md
+++ b/doc/misc/changelog.md
@@ -5,6 +5,12 @@ updates to latest `fwdpp` version, etc.
 
 ## 0.20.1
 
+Bug fixes
+
+* Fix attribute errors when validating recombination intervals.
+  PR {pr}`1155`
+  Issue {issue}`1153`
+
 Build system and CI.
 
 * Update pip usage and ensure reproducible builds of rust code.
diff --git a/fwdpy11/_evolvets.py b/fwdpy11/_evolvets.py
index 8ac68566e..a94cc4829 100644
--- a/fwdpy11/_evolvets.py
+++ b/fwdpy11/_evolvets.py
@@ -191,12 +191,30 @@ def evolvets(
             isinstance(
                 r, fwdpy11._fwdpy11.NonPoissonCrossoverGenerator) or \
             isinstance(r, fwdpy11.Region), f"{type(r)}"
-        if r.beg < 0:
-            raise ValueError(f"{r} has begin value < 0.0")
-        if r.end > pop.tables.genome_length:
-            raise ValueError(
-                f"{r} has end value >= genome length of {pop.tables.genome_length}"
-            )
+        if hasattr(r, "beg") and hasattr(r, "end"):
+            if r.beg < 0:
+                raise ValueError(f"{r} has begin value < 0.0")
+            if r.end > pop.tables.genome_length:
+                raise ValueError(
+                    f"{r} has end value >= genome length of {pop.tables.genome_length}"
+                )
+        elif hasattr(r, "position"):
+            if r.position < 0:
+                raise ValueError(f"{r} has position value < 0.0")
+            if r.position >= pop.tables.genome_length:
+                raise ValueError(
+                    f"{r} has position value >= genome length of {pop.tables.genome_length}"
+                )
+        elif hasattr(r, "regions"):
+            for i in r.regions:
+                if i.beg < 0:
+                    raise ValueError(f"{r} has begin value < 0.0")
+                if i.end > pop.tables.genome_length:
+                    raise ValueError(
+                        f"{r} has end value >= genome length of {pop.tables.genome_length}"
+                    )
+        else:
+            raise TypeError(f"unexpceted type: {type(r)}")
 
     if recorder is None:
         from ._fwdpy11 import NoAncientSamples
@@ -225,7 +243,6 @@ def evolvets(
 
     fwdpy11._validate_regions(params.sregions, pop.tables.genome_length)
     fwdpy11._validate_regions(params.nregions, pop.tables.genome_length)
-    fwdpy11._validate_regions(params.recregions, pop.tables.genome_length)
 
     pneutral = 0.0
     if params.rates.neutral_mutation_rate + params.rates.selected_mutation_rate > 0.0:
diff --git a/tests/demes-spec b/tests/demes-spec
index 44803a99f..402bbbca1 160000
--- a/tests/demes-spec
+++ b/tests/demes-spec
@@ -1 +1 @@
-Subproject commit 44803a99f28209560948acab1a6b4106ec2c0283
+Subproject commit 402bbbca16ba9d0da520ab6b17089dc804708aae
diff --git a/tests/test_ModelParams_with_recregions.py b/tests/test_ModelParams_with_recregions.py
index 786e0a56c..3e2f85866 100644
--- a/tests/test_ModelParams_with_recregions.py
+++ b/tests/test_ModelParams_with_recregions.py
@@ -17,6 +17,7 @@
 # along with fwdpy11.  If not, see <http://www.gnu.org/licenses/>.
 #
 
+import demes
 import pytest
 
 import fwdpy11
@@ -30,6 +31,30 @@ def _validate_expected(regions, numbers):
         i, fwdpy11.NonPoissonCrossoverGenerator)]) == numbers[1]
 
 
+def roundtrip(regions):
+    yaml = """
+    time_units: generations
+    demes:
+    - name: A
+      epochs:
+        - {end_time: 0, start_size: 10}
+    """
+    graph = demes.loads(yaml)
+    demog = fwdpy11.discrete_demography.from_demes(graph, burnin=1)
+    pdict = {"recregions": regions,
+             "nregions": [],
+             "sregions": [],
+             "rates": (0, 0, None),
+             "gvalue": fwdpy11.Multiplicative(2.),
+             "simlen": 10,
+             "demography": demog,
+             }
+    params = fwdpy11.ModelParams(**pdict)
+    pop = fwdpy11.DiploidPopulation(10, 2)
+    rng = fwdpy11.GSLrng(123)
+    fwdpy11.evolvets(rng, pop, params, 100)
+
+
 # The setup is ([regions], (number poisson, number non-poisson))
 # The latter is used to mimic the internal idiom that we
 # use to set up the genetic map to send to C++
@@ -52,11 +77,4 @@ def _validate_expected(regions, numbers):
 def test_model_params_with_recregions(data):
     regions, expected = data
     _validate_expected(regions, expected)
-    pdict = {"recregions": regions,
-             "nregions": [],
-             "sregions": [],
-             "rates": (0, 0, None),
-             "gvalue": fwdpy11.Multiplicative(2.),
-             "simlen": 10,
-             }
-    _ = fwdpy11.ModelParams(**pdict)
+    roundtrip(regions)