Skip to content

Commit

Permalink
Bump pybind11 to 2.10.0
Browse files Browse the repository at this point in the history
Fixes for py::make_iterator:

* Add asserts to hit #954 without triggering segfault
  in Python tests
* Add runtime checks of internal iterator validity.
* Use std::move for py::make_iterator.
  This is required for pybind11 2.10 so that we don't
  segfault.
* See GitHub issue #954
  • Loading branch information
molpopgen committed Sep 23, 2022
1 parent 8b4e92e commit 22dde32
Show file tree
Hide file tree
Showing 6 changed files with 23 additions and 7 deletions.
4 changes: 2 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ endif()

find_package(pybind11)
message(STATUS "Found pybind11: ${pybind11_VERSION}")
if(${pybind11_VERSION} VERSION_LESS '2.4.3')
message(FATAL_ERROR "pybind11 version must be >= '2.4.3'")
if(${pybind11_VERSION} VERSION_LESS '2.10.0')
message(FATAL_ERROR "pybind11 version must be >= '2.10.0'")
endif()
add_definitions(-DPYBIND11_VERSION="${pybind11_VERSION}")

Expand Down
13 changes: 11 additions & 2 deletions fwdpy11/src/ts/TreeIterator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,9 @@ class tree_visitor_wrapper
{
++end_of_range;
}
if (end_of_range > end_of_sites) {
throw std::runtime_error("invalid end of site range");
}
return std::make_pair(current_site, end_of_range);
}

Expand All @@ -250,6 +253,9 @@ class tree_visitor_wrapper
{
++current_site;
}
if (current_site > end_of_sites) {
throw std::runtime_error("invalid end of site range during mutation iteration");
}
while ((current_site < end_of_sites) && (current_mutation < end_of_mutations)
&& (first_site + current_mutation->site)->position
!= current_site->position)
Expand All @@ -272,6 +278,9 @@ class tree_visitor_wrapper
{
++end_of_range;
}
if (end_of_range > end_of_mutations) {
throw std::runtime_error("invalid end of mutation range");
}
return std::make_pair(current_mutation, end_of_range);
}
};
Expand Down Expand Up @@ -352,14 +361,14 @@ init_tree_iterator(py::module& m)
"_sites",
[](tree_visitor_wrapper& self) {
auto rv = self.get_sites_on_current_tree();
return py::make_iterator(rv.first, rv.second);
return py::make_iterator(std::move(rv.first), std::move(rv.second));
},
py::keep_alive<0, 1>())
.def(
"_mutations",
[](tree_visitor_wrapper& self) {
auto r = self.get_mutations_on_current_tree();
return py::make_iterator(r.first, r.second);
return py::make_iterator(std::move(r.first), std::move(r.second));
},
py::keep_alive<0, 1>());
}
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[build-system]
# We need setup.cfg support, which setuptools indtroduced in 30.3.0.
requires = ["setuptools>=30.3.0", "wheel", "setuptools_scm", "pybind11[global]==2.9.1"]
requires = ["setuptools>=30.3.0", "wheel", "setuptools_scm", "pybind11[global]==2.10.0"]

[tool.setuptools_scm]
write_to = "fwdpy11/_version.py"
2 changes: 1 addition & 1 deletion requirements/conda_minimal_deps.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
intervaltree
pybind11==2.9.1
pybind11==2.10.0
numpy
scipy
attrs>=0.19.2
Expand Down
2 changes: 1 addition & 1 deletion requirements/development.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# NOTE: any pinning should also be coordinated
# with requirements.in and doc/requirements.in
# and may require regenerating the .txt files.
pybind11[global]==2.9.1
pybind11[global]==2.10.0
intervaltree
numpy
scipy
Expand Down
7 changes: 7 additions & 0 deletions tests/test_tree_sequences_with_neutral_mutations.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,15 @@ def test_mutation_counts_with_indexing_suppressed_no_neutral_muts_in_genomes(sel
)
ti = fwdpy11.TreeIterator(self.pop.tables, [i for i in range(2 * self.pop.N)])
mc = _count_mutations_from_diploids(self.pop)
# This assert was added when working
# on GitHub isuse 954
for m in pop2.tables.mutations:
assert m.key < len(pop2.tables.mutations)
for t in ti:
for m in t.mutations():
# This assert was added when working
# on GitHub isuse 954
assert m.key < len(pop2.mutations)
# Have to skip neutral mutations b/c they won't
# end up in mc b/c it is obtained from genomes
if pop2.mutations[m.key].neutral is False:
Expand Down

0 comments on commit 22dde32

Please sign in to comment.