From 70b55b6b3e90f39d24db767ebb3c7c4cbc2d7773 Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Mon, 19 Oct 2026 11:21:15 -0400 Subject: [PATCH] feat: let constrain_as_space_group take a generator object --- news/structure-api.rst | 23 ++++++++ src/diffpy/cmistructure/bvsrestraint.py | 7 ++- src/diffpy/cmistructure/sgconstraints.py | 57 ++++++++++++------ tests/conftest.py | 20 +++++++ tests/test_sgconstraints.py | 73 ++++++++++++++++++------ 5 files changed, 143 insertions(+), 37 deletions(-) create mode 100644 news/structure-api.rst diff --git a/news/structure-api.rst b/news/structure-api.rst new file mode 100644 index 0000000..6e7106e --- /dev/null +++ b/news/structure-api.rst @@ -0,0 +1,23 @@ +**Added:** + +* ``constrain_as_space_group`` also accepts a generator, such as a ``PDFGenerator``, whose ``.phase`` is the structure parameter set, for example ``constrain_as_space_group(generator, "Fm-3m")``. + +**Changed:** + +* + +**Deprecated:** + +* + +**Removed:** + +* + +**Fixed:** + +* + +**Security:** + +* diff --git a/src/diffpy/cmistructure/bvsrestraint.py b/src/diffpy/cmistructure/bvsrestraint.py index da0fb89..3fffbee 100644 --- a/src/diffpy/cmistructure/bvsrestraint.py +++ b/src/diffpy/cmistructure/bvsrestraint.py @@ -106,9 +106,12 @@ def penalty(self, w=1.0): return penalty def _validate(self): - """This evaluates the calculator. + """Evaluate the calculator to check that it is valid. - Raises SrFitError if validation fails. + Raises + ------ + SrFitError + If the penalty cannot be computed. """ from numpy import nan diff --git a/src/diffpy/cmistructure/sgconstraints.py b/src/diffpy/cmistructure/sgconstraints.py index 58268aa..df35631 100644 --- a/src/diffpy/cmistructure/sgconstraints.py +++ b/src/diffpy/cmistructure/sgconstraints.py @@ -22,6 +22,7 @@ import numpy +from diffpy.cmistructure.basestructureparset import BaseStructureParSet from diffpy.srfit.fitbase.parameter import ParameterProxy from diffpy.srfit.fitbase.recipeorganizer import RecipeContainer @@ -51,8 +52,9 @@ def constrain_as_space_group( Parameters ---------- - phase : BaseStructureParSet - The structure ParameterSet to constrain. + phase : BaseStructureParSet or object with a .phase attribute + The structure ParameterSet to constrain, or a generator such as a + PDFGenerator whose ``.phase`` is the structure ParameterSet. spacegroup : int, str or diffpy.structure.spacegroups.SpaceGroup The space group number, symbol or SpaceGroup instance. scatterers : list of ParameterSet, optional @@ -102,9 +104,16 @@ def constrain_as_space_group( Cubic b and c are constrained to a, and alpha, beta and gamma are fixed to 90. + + Raises + ------ + TypeError + If ``phase`` is neither a structure ParameterSet nor an object + whose ``.phase`` is one. """ from diffpy.structure.spacegroups import SpaceGroup, get_space_group + phase = _get_structure_parameter_set(phase) space_group = spacegroup if not isinstance(spacegroup, SpaceGroup): space_group = get_space_group(spacegroup) @@ -134,10 +143,9 @@ def _constrain_as_space_group( ): """Restricted interface to constrain_as_space_group. - Arguments: As constrain_as_space_group, except - ----------------------------------------------- - sg - diffpy.structure.spacegroups.SpaceGroup instance + The arguments are as for constrain_as_space_group, except that + ``phase`` must be a structure ParameterSet and ``space_group`` must + be a diffpy.structure.spacegroups.SpaceGroup instance. """ from diffpy.structure.symmetryutilities import stdUsymbols @@ -160,6 +168,19 @@ def _constrain_as_space_group( return sgp +def _get_structure_parameter_set(phase): + if isinstance(phase, BaseStructureParSet): + return phase + generator_phase = getattr(phase, "phase", None) + if isinstance(generator_phase, BaseStructureParSet): + return generator_phase + raise TypeError( + "Expected a structure parameter set or a generator with a .phase " + f"attribute, got {type(phase).__name__}. " + "Please pass generator.phase or the generator itself." + ) + + # End constrain_as_space_group @@ -769,21 +790,21 @@ def _makeconstraint(parname, formula, scatterer, index, ns={}): Parameters ---------- - parname - Name of parameter - formula - Constraint formula - scatterer - scatterer containing par of parname - idx - Index to identify scatterer from which par comes - ns - namespace to draw extra names from (default {}) + parname : str + The name of the Parameter. + formula : str + The constraint formula. + scatterer : ParameterSet + The scatterer containing the Parameter named ``parname``. + index : int + The index that identifies the scatterer the Parameter comes from. + ns : dict, optional + The namespace to draw extra names from (default {}). Returns ------- - par - Returns the parameter if it is free. + Parameter or None + The Parameter if it is free, otherwise None. """ parameter = scatterer.get(parname) diff --git a/tests/conftest.py b/tests/conftest.py index eab6206..584fbbb 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -6,6 +6,9 @@ import pytest +from diffpy.cmistructure.diffpyparset import DiffpyStructureParSet +from diffpy.structure import Structure + @pytest.fixture def user_filesystem(tmp_path): @@ -47,3 +50,20 @@ def _capturestdout(f, *args, **kwargs): return fp.getvalue() return _capturestdout + + +class GeneratorStandIn: + """Stand in for a generator, such as a PDFGenerator, that holds a + structure parameter set as ``.phase``.""" + + def __init__(self, phase): + self.phase = phase + + +@pytest.fixture +def lamno3_generator(datafile): + structure = Structure() + structure.read(datafile("LaMnO3.stru")) + parameter_set = DiffpyStructureParSet("LaMnO3", structure) + generator = GeneratorStandIn(parameter_set) + return generator diff --git a/tests/test_sgconstraints.py b/tests/test_sgconstraints.py index 24ebeb8..3707d3e 100644 --- a/tests/test_sgconstraints.py +++ b/tests/test_sgconstraints.py @@ -18,8 +18,20 @@ ############################################################################## """Tests space group constraints.""" +import re + import numpy import pytest +from pyobjcryst.atom import Atom +from pyobjcryst.crystal import Crystal +from pyobjcryst.scatteringpower import ScatteringPowerAtom + +from diffpy.cmistructure.diffpyparset import DiffpyStructureParSet +from diffpy.cmistructure.objcrystparset import ObjCrystCrystalParSet +from diffpy.cmistructure.sgconstraints import constrain_as_space_group +from diffpy.srfit.fitbase.fitrecipe import FitRecipe +from diffpy.structure import Structure +from diffpy.structure.spacegroups import get_space_group # ---------------------------------------------------------------------------- @@ -31,8 +43,6 @@ def test_ObjCryst_constrain_space_group(): diffpy.cmistructure.sgconstraints, which is performed automatically when an ObjCrystCrystalParSet is created. """ - from diffpy.cmistructure.objcrystparset import ObjCrystCrystalParSet - pi = numpy.pi occryst = makeLaMnO3() @@ -106,7 +116,6 @@ def test_ObjCryst_constrain_space_group(): mn.add_constraint(mn.z, "x") # Nor can we make them into variables - from diffpy.srfit.fitbase.fitrecipe import FitRecipe f = FitRecipe() with pytest.raises(ValueError): @@ -117,9 +126,6 @@ def test_ObjCryst_constrain_space_group(): def test_DiffPy_constrain_as_space_group(datafile): """Test the constrain_as_space_group function.""" - from diffpy.cmistructure.diffpyparset import DiffpyStructureParSet - from diffpy.cmistructure.sgconstraints import constrain_as_space_group - structure = makeLaMnO3_P1(datafile) parameter_set = DiffpyStructureParSet("LaMnO3", structure) @@ -223,10 +229,6 @@ def _alltests(parameter): def test_constrain_as_space_group_args(datafile): """Test the arguments processing of constrain_as_space_group function.""" - from diffpy.cmistructure.diffpyparset import DiffpyStructureParSet - from diffpy.cmistructure.sgconstraints import constrain_as_space_group - from diffpy.structure.spacegroups import get_space_group - # C1: The space group is given as a symbol or as a SpaceGroup object. # Expected: Both create the same space group Parameters. structure = makeLaMnO3_P1(datafile) @@ -247,6 +249,50 @@ def test_constrain_as_space_group_args(datafile): return +def test_constrain_as_space_group_generator(lamno3_generator): + # Test that constrain_as_space_group accepts a generator in place of + # its structure parameter set. + # C1: A generator holding the P1 LaMnO3 parameter set as .phase is + # constrained to the orthorhombic space group P b n m. + # Expected: Only the lattice lengths a, b and c of its .phase are left + # free. + space_group_parameters = constrain_as_space_group( + lamno3_generator, "P b n m" + ) + actual_free_lattice_names = [ + parameter.name for parameter in space_group_parameters.latpars + ] + expected_free_lattice_names = ["a", "b", "c"] + assert actual_free_lattice_names == expected_free_lattice_names + + +@pytest.mark.parametrize( + "input_phase, expected_type_name", + [ + # Test that constrain_as_space_group rejects objects that are + # neither a structure parameter set nor a generator holding one. + # C1: A diffpy.structure Structure is passed instead of its + # parameter set. + # Expected: A TypeError names the Structure type and explains what + # to pass instead. + (Structure(), "Structure"), + # C2: A file name is passed instead of a parameter set. + # Expected: A TypeError names the str type and explains what to + # pass instead. + ("LaMnO3.stru", "str"), + ], +) +def test_constrain_as_space_group_bad_phase(input_phase, expected_type_name): + + expected_msg = ( + "Expected a structure parameter set or a generator with a .phase " + f"attribute, got {expected_type_name}. " + "Please pass generator.phase or the generator itself." + ) + with pytest.raises(TypeError, match=re.escape(expected_msg)): + constrain_as_space_group(input_phase, "P b n m") + + # The space group containers keep diffpy.srfit's names, since user scripts # and parameter paths such as "sgpars.latpars.a" rely on them. @pytest.mark.parametrize( @@ -264,8 +310,6 @@ def test_constrain_as_space_group_args(datafile): def test_space_group_parameter_container_names( datafile, input_container_attribute, expected_container_name ): - from diffpy.cmistructure.diffpyparset import DiffpyStructureParSet - from diffpy.cmistructure.sgconstraints import constrain_as_space_group parameter_set = DiffpyStructureParSet("LaMnO3", makeLaMnO3_P1(datafile)) space_group_parameters = constrain_as_space_group(parameter_set, "P b n m") @@ -278,7 +322,6 @@ def test_objcryst_space_group_parameters_name(): # C1: The space group Parameters are read from an ObjCrystCrystalParSet. # Expected: They are available as .sgpars, in a container named # "sgpars". - from diffpy.cmistructure.objcrystparset import ObjCrystCrystalParSet occryst = makeLaMnO3() structure = ObjCrystCrystalParSet(occryst.GetName(), occryst) @@ -288,7 +331,6 @@ def test_objcryst_space_group_parameters_name(): def makeLaMnO3_P1(datafile): - from diffpy.structure import Structure structure = Structure() structure.read(datafile("LaMnO3.stru")) @@ -296,9 +338,6 @@ def makeLaMnO3_P1(datafile): def makeLaMnO3(): - from pyobjcryst.atom import Atom - from pyobjcryst.crystal import Crystal - from pyobjcryst.scatteringpower import ScatteringPowerAtom pi = numpy.pi # It appears that ObjCryst only supports standard symbols