From 97ad3fa0cb40ca7583cfa59ac9ee30db56ae02cc Mon Sep 17 00:00:00 2001 From: Lester Hedges Date: Wed, 1 Jul 2026 14:09:53 +0100 Subject: [PATCH] Fix restraint pickling and sire.mm/mol circular import ordering. --- doc/source/changelog.rst | 9 +++++++ src/sire/mm/__init__.py | 19 +++++++++++--- src/sire/restraints/_restraints.py | 3 +-- tests/restraints/test_boresch.py | 32 ++++++++++++++++++++++- tests/stream/test_pickle.py | 41 ++++++++++++++++++++++++++++++ 5 files changed, 97 insertions(+), 7 deletions(-) create mode 100644 tests/stream/test_pickle.py diff --git a/doc/source/changelog.rst b/doc/source/changelog.rst index f46abd7fd..098551274 100644 --- a/doc/source/changelog.rst +++ b/doc/source/changelog.rst @@ -17,6 +17,15 @@ organisation on `GitHub `__. * Please add an item to this CHANGELOG for any new features or bug fixes when creating a PR. +* Fixed ``sire.restraints.boresch()`` setting a dynamic ``_use_pbc`` Python attribute on + the returned ``BoreschRestraints`` instead of calling ``set_uses_pbc()``, which broke + pickling (e.g. for ``multiprocessing``/``ProcessPoolExecutor``) and meant the flag did + not survive a ``sire.stream.save``/``load`` round-trip. + +* Fixed a module load-order bug where ``sire.mm`` failed to import (``cannot import + name '_fix_siremm' from 'sire.mm'``) if it was the first ``sire`` submodule touched in + a process, due to ``use_new_api()`` being called before ``_fix_siremm`` was defined. + `2026.1.0 `__ - June 2026 ----------------------------------------------------------------------------------------- diff --git a/src/sire/mm/__init__.py b/src/sire/mm/__init__.py index de5dec3ca..172c42fdf 100644 --- a/src/sire/mm/__init__.py +++ b/src/sire/mm/__init__.py @@ -36,10 +36,6 @@ from ..legacy import MM as _MM -from .. import use_new_api as _use_new_api - -_use_new_api() - AngleRestraint = _MM.AngleRestraint AngleRestraints = _MM.AngleRestraints @@ -217,3 +213,18 @@ def _fix_siremm(): _fix_siremm() except ImportError: pass + +# This must be called after _fix_siremm() above, not at the top of this +# module (as in most other new-API submodules). use_new_api() eagerly loads +# every other lazily-loaded new-API submodule, including sire.mol - and +# sire.mol's own module-level code does 'from ..mm import _fix_siremm'. If +# sire.mm is the first Sire submodule touched in a process, calling +# use_new_api() before _fix_siremm is defined means that reentrant load of +# sire.mol fails with 'cannot import name _fix_siremm from sire.mm', which +# then cascades into 'sire.mm could not be loaded' via the lazy_import +# wrapper. Nothing above this point depends on use_new_api() having run +# (it only pythonizes names already pulled directly from the raw legacy +# _MM module), so it is safe to defer to here. +from .. import use_new_api as _use_new_api + +_use_new_api() diff --git a/src/sire/restraints/_restraints.py b/src/sire/restraints/_restraints.py index 37bc78723..51a97b919 100644 --- a/src/sire/restraints/_restraints.py +++ b/src/sire/restraints/_restraints.py @@ -447,7 +447,7 @@ def boresch( b = BoreschRestraints(name, b) # Set the use_pbc flag. - b._use_pbc = use_pbc + b.set_uses_pbc(use_pbc) return b @@ -876,7 +876,6 @@ def morse_potential( for bond in changed_bonds: bond_name, length0, length1, k0, k1 = bond if k1 == 0 or k0 == 0: - # If the bond is being created (k0 == 0), then we should # use the parameters from the final state (length1, k1). # If the bond is being annihilated (k1 == 0), then we don't diff --git a/tests/restraints/test_boresch.py b/tests/restraints/test_boresch.py index b134ee975..2ca8ca2cc 100644 --- a/tests/restraints/test_boresch.py +++ b/tests/restraints/test_boresch.py @@ -1,6 +1,7 @@ +import pickle + import pytest -import sire as sr from sire.restraints import boresch # Valid Boresch restraint parameters. @@ -203,6 +204,35 @@ def test_boresch_restraint_params(thrombin_complex): assert boresch_restraint.phi0()[2].value() == 1.4147 +def test_boresch_restraints_pickles_ok(thrombin_complex): + """ + Regression test: sire.restraints.boresch() used to set a dynamic + '_use_pbc' Python attribute on the returned BoreschRestraints instead of + calling set_uses_pbc(), which broke pickling (fixed). + """ + boresch_restraints = boresch( + thrombin_complex, + receptor=thrombin_complex["protein"][ + BORESCH_PARAMS_DEFAULT["receptor_selection"] + ], + ligand=thrombin_complex["resname LIG"][ + BORESCH_PARAMS_DEFAULT["ligand_selection"] + ], + kr=BORESCH_PARAMS_DEFAULT["kr"], + ktheta=BORESCH_PARAMS_DEFAULT["ktheta"], + kphi=BORESCH_PARAMS_DEFAULT["kphi"], + r0=BORESCH_PARAMS_DEFAULT["r0"], + theta0=BORESCH_PARAMS_DEFAULT["theta0"], + phi0=BORESCH_PARAMS_DEFAULT["phi0"], + name=BORESCH_PARAMS_DEFAULT["name"], + ) + + data = pickle.dumps(boresch_restraints) + reloaded = pickle.loads(data) + + assert reloaded[0].kr().value() == boresch_restraints[0].kr().value() + + @pytest.mark.parametrize( ( "receptor_selection", diff --git a/tests/stream/test_pickle.py b/tests/stream/test_pickle.py new file mode 100644 index 000000000..9a03cb6fe --- /dev/null +++ b/tests/stream/test_pickle.py @@ -0,0 +1,41 @@ +import pickle + +import pytest + +import sire as sr + + +def test_dynamic_attribute_breaks_pickling(): + """ + General, still-open Sire issue: sire_pickle_suite (wrapper/Qt/ + qdatastream.hpp) never overrides pickle_suite::getstate_manages_dict(), + so any object using it that also has a dynamic Python attribute set on + it (populating __dict__) fails to pickle with 'Incomplete pickle support + (__getstate_manages_dict__ not set)', even though the object's own + QDataStream-based serialisation (sire.stream.save/load) is unaffected. + """ + from sire.mm import BoreschRestraint, BoreschRestraints + + b = BoreschRestraint( + receptor=[1574, 1554, 1576], + ligand=[4, 3, 5], + r0=sr.u("7.687 A"), + theta0=[sr.u("1.3031 rad"), sr.u("1.4777 rad")], + phi0=[sr.u("2.5569 rad"), sr.u("2.9359 rad"), sr.u("1.4147 rad")], + kr=sr.u("6.2012 kcal mol-1 A-2"), + ktheta=[sr.u("28.7685 kcal mol-1 rad-2"), sr.u("24.8204 kcal mol-1 rad-2")], + kphi=[ + sr.u("59.8626 kcal mol-1 rad-2"), + sr.u("0.7923 kcal mol-1 rad-2"), + sr.u("55.1775 kcal mol-1 rad-2"), + ], + ) + restraints = BoreschRestraints(b) + + # Pickles fine before any dynamic attribute is set. + pickle.dumps(restraints) + + restraints._some_dynamic_attribute = True + + with pytest.raises(RuntimeError, match="Incomplete pickle support"): + pickle.dumps(restraints)