fix: fix .show() crash when restraints weren't added with add_soft_bounds (#227)

Author: cadenmyers13

news/fix-show-restraints.rst

@@ -0,0 +1,23 @@
+**Added:**
+
+* <news item>
+
+**Changed:**
+
+* <news item>
+
+**Deprecated:**
+
+* <news item>
+
+**Removed:**
+
+* <news item>
+
+**Fixed:**
+
+* ``show()`` no longer fails when the recipe holds BVS or ObjCryst molecule restraints, or a ``Restraint`` added with ``register_soft_bounds``.
+
+**Security:**
+
+* <news item>

src/diffpy/srfit/fitbase/recipeorganizer.py

@@ -1681,20 +1681,7 @@ def _format_restraints(self):
             The list of formatted lines displaying the defined
             restraints. Empty list when no restraints were defined.
         """
-        rset = self._get_restraints()
-        rlines = []
-        for res in rset:
-            line = (
-                "%s: lower_bound = %f, upper_bound = %f, sig = %f, scaled = %s"
-                % (
-                    res.eqstr,
-                    res.lower_bound,
-                    res.upper_bound,
-                    res.sig,
-                    res.scaled,
-                )
-            )
-            rlines.append(line)
+        rlines = [str(restraint) for restraint in self._get_restraints()]
         rlines.sort(key=numstr)
         return rlines
 

src/diffpy/srfit/fitbase/restraint.py

@@ -41,6 +41,9 @@ class Restraint(Validatable):
     eq : Equation
         The equation whose evaluation is compared against the restraint
         bounds.
+    eqstr : str
+        The text shown for the restrained equation. It is the name of
+        ``eq`` unless set otherwise, as ``add_soft_bounds`` does.
     lower_bound : float
         The lower bound on the restraint evaluation (default -inf).
     upper_bound : float
@@ -75,12 +78,21 @@ def __init__(
             (default False).
         """
         self.eq = eq
+        self.eqstr = eq.name
         self.lower_bound = float(lower_bound)
         self.upper_bound = float(upper_bound)
         self.sig = float(sig)
         self.scaled = bool(scaled)
         return
 
+    def __str__(self):
+        """Return the equation with its bounds and settings."""
+        return (
+            f"{self.eqstr}: lower_bound = {self.lower_bound:f}, "
+            f"upper_bound = {self.upper_bound:f}, sig = {self.sig:f}, "
+            f"scaled = {self.scaled}"
+        )
+
     def penalty(self, w=1.0):
         """Calculate the penalty of the restraint.
 

src/diffpy/srfit/structure/bvsrestraint.py

@@ -66,6 +66,13 @@ def __init__(self, parset, sig=1, scaled=False):
         self.scaled = bool(scaled)
         return
 
+    def __str__(self):
+        """Return the structure name with the restraint settings."""
+        return (
+            f"BVS of {self._parset.name}: sig = {self.sig:f}, "
+            f"scaled = {self.scaled}"
+        )
+
     def penalty(self, w=1.0):
         """Calculate the penalty of the restraint.
 

src/diffpy/srfit/structure/objcrystparset.py

@@ -898,6 +898,24 @@ def __init__(self, res, scaled=False):
         self.scaled = scaled
         return
 
+    def __str__(self):
+        """Return the restrained atoms with the restraint settings."""
+        res = self.res
+        if hasattr(res, "GetLength0"):
+            target = (
+                f"length = {res.GetLength0():f}, "
+                f"sigma = {res.GetLengthSigma():f}, "
+                f"delta = {res.GetLengthDelta():f}"
+            )
+        else:
+            target = (
+                f"angle = {res.GetAngle0():f}, "
+                f"sigma = {res.GetAngleSigma():f}, "
+                f"delta = {res.GetAngleDelta():f}"
+            )
+        restraint_str = f"{res.GetName()}: {target}, scaled = {self.scaled}"
+        return restraint_str
+
     def penalty(self, w=1.0):
         """Calculate the penalty of the restraint.
 

tests/test_diffpyparset.py

@@ -18,6 +18,7 @@
 import unittest
 
 import numpy as np
+import pytest
 
 from diffpy.srfit.structure.diffpyparset import DiffpyStructureParSet
 
@@ -127,6 +128,27 @@ def test_pickling():
     return
 
 
+def test_show_bvs_restraint(capturestdout, diffpy_srreal_available):
+    # C1: A scaled BVS restraint with sig = 0.5 is added to the structure
+    # nacl.
+    # Expected: The Restraints section lists it as the BVS of nacl with its
+    # sig and scaling.
+    if not diffpy_srreal_available:
+        pytest.skip("diffpy.srreal package not available")
+    from diffpy.structure import Atom, Lattice, Structure
+
+    lattice = Lattice(5.6, 5.6, 5.6, 90, 90, 90)
+    atoms = [Atom("Na", [0, 0, 0]), Atom("Cl", [0.5, 0.5, 0.5])]
+    parameter_set = DiffpyStructureParSet(
+        "nacl", Structure(atoms, lattice=lattice)
+    )
+    parameter_set.restrainBVS(sig=0.5, scaled=True)
+    lines = capturestdout(parameter_set.show).splitlines()
+    actual_restraint_lines = lines[lines.index("Restraints") + 2 :]
+    expected_restraint_lines = ["BVS of nacl: sig = 0.500000, scaled = True"]
+    assert actual_restraint_lines == expected_restraint_lines
+
+
 # End of class TestParameterAdapter
 
 if __name__ == "__main__":

tests/test_objcrystparset.py

@@ -318,6 +318,57 @@ def testExplicitBondLengthRestraints(self):
 
         return
 
+    def test_show_explicit_molecule_restraints(self, capturestdout):
+        # C1: A bond length, a bond angle and a dihedral angle restraint are
+        # added through the ObjCrystMoleculeParSet.
+        # Expected: The Restraints section lists each restraint by its atom
+        # names with its target value, sigma, delta and scaling.
+        cryst = ObjCrystCrystalParSet("bucky", self.occryst)
+        molecule = cryst.c60
+        atoms = molecule.atoms
+        molecule.restrainBondLength(atoms[0], atoms[5], 3.3, 0.1, 0.2)
+        molecule.restrainBondAngle(atoms[0], atoms[5], atoms[7], 1.0, 0.1, 0.2)
+        molecule.restrainDihedralAngle(
+            atoms[0], atoms[5], atoms[7], atoms[8], 2.0, 0.1, 0.2
+        )
+        lines = capturestdout(cryst.show, textwidth=0).splitlines()
+        actual_restraint_lines = set(lines[lines.index("Restraints") + 2 :])
+        expected_restraint_lines = {
+            "C0-C5: length = 3.300000, sigma = 0.100000, delta = 0.200000, "
+            "scaled = False",
+            "C0-C5-C7: angle = 1.000000, sigma = 0.100000, delta = 0.200000, "
+            "scaled = False",
+            "C0-C5-C7-C8: angle = 2.000000, sigma = 0.100000, "
+            "delta = 0.200000, scaled = False",
+        }
+        assert actual_restraint_lines == expected_restraint_lines
+
+    def test_show_implicit_molecule_restraints(self, capturestdout):
+        # C1: A bond length, a bond angle and a dihedral angle restraint are
+        # defined on the pyobjcryst Molecule and wrapped with
+        # wrapRestraints.
+        # Expected: The Restraints section lists each restraint by its atom
+        # names with its target value, sigma, delta and scaling.
+        ocmol = self.ocmol
+        ocmol.AddBond(ocmol[0], ocmol[5], 3.3, 0.1, 0.2)
+        ocmol.AddBondAngle(ocmol[0], ocmol[5], ocmol[7], 1.0, 0.1, 0.2)
+        ocmol.AddDihedralAngle(
+            ocmol[0], ocmol[5], ocmol[7], ocmol[8], 2.0, 0.1, 0.2
+        )
+        cryst = ObjCrystCrystalParSet("bucky", self.occryst)
+        cryst.c60.wrapRestraints()
+        lines = capturestdout(cryst.show, textwidth=0).splitlines()
+        actual_restraint_lines = set(lines[lines.index("Restraints") + 2 :])
+        expected_restraint_lines = {
+            "C0-C5: length = 3.300000, sigma = 0.100000, delta = 0.200000, "
+            "scaled = False",
+            "C0-C5-C7: angle = 1.000000, sigma = 0.100000, delta = 0.200000, "
+            "scaled = False",
+            "C0-C5-C7-C8: angle = 2.000000, sigma = 0.100000, "
+            "delta = 0.200000, scaled = False",
+        }
+        assert actual_restraint_lines == expected_restraint_lines
+
     def testExplicitBondAngleRestraints(self):
         """Test the structure with explicit bond angles.
 

tests/test_recipeorganizer.py

@@ -21,13 +21,15 @@
 import pytest
 
 from diffpy.srfit import Calculator
+from diffpy.srfit.equation import Equation
 from diffpy.srfit.equation.builder import EquationFactory
 from diffpy.srfit.fitbase.parameter import Parameter
 from diffpy.srfit.fitbase.recipeorganizer import (
     RecipeContainer,
     RecipeOrganizer,
     get_equation_from_string,
 )
+from diffpy.srfit.fitbase.restraint import Restraint
 from diffpy.utils._deprecator import deprecated
 
 
@@ -901,6 +903,38 @@ def test_show_pattern(capturestdout, input_stage, input_pattern):
     assert actual_output == expected_output
 
 
+def test_show_added_soft_bounds(capturestdout):
+    # C1: The soft bound 0 < p < 2 is added with add_soft_bounds.
+    # Expected: The Restraints section lists it by the Parameter name p.
+    organizer = _make_organizer()
+    organizer._new_parameter("p", 1)
+    organizer.add_soft_bounds("p", lower_bound=0, upper_bound=2)
+    lines = capturestdout(organizer.show, textwidth=0).splitlines()
+    actual_restraint_lines = lines[lines.index("Restraints") + 2 :]
+    expected_restraint_lines = [
+        "p: lower_bound = 0.000000, upper_bound = 2.000000, "
+        "sig = 1.000000, scaled = False"
+    ]
+    assert actual_restraint_lines == expected_restraint_lines
+
+
+def test_show_registered_soft_bounds(capturestdout):
+    # C1: A Restraint 0 < p < 2 made directly from an Equation on p is
+    # added with register_soft_bounds.
+    # Expected: The Restraints section lists it by its equation name eq_p.
+    organizer = _make_organizer()
+    parameter = organizer._new_parameter("p", 1)
+    restraint = Restraint(Equation(root=parameter), 0, 2)
+    organizer.register_soft_bounds(restraint)
+    lines = capturestdout(organizer.show, textwidth=0).splitlines()
+    actual_restraint_lines = lines[lines.index("Restraints") + 2 :]
+    expected_restraint_lines = [
+        "eq_p: lower_bound = 0.000000, upper_bound = 2.000000, "
+        "sig = 1.000000, scaled = False"
+    ]
+    assert actual_restraint_lines == expected_restraint_lines
+
+
 def _make_iterpars_tree():
     """Build a small hierarchy for iterate_over_parameters tests."""
     root = RecipeContainer("root")