From 6b2270221556aaeb21abc4876a4a9da8a386b1dd Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Mon, 2 Nov 2026 12:23:13 -0500 Subject: [PATCH 1/5] test: add tests for set_structure taking a ParameterSet or a structure file path Tests in tests/test_generators.py for BasePDFGenerator and DebyePDFGenerator.set_structure accepting a diffpy.cmistructure ParameterSet (kept under its own name; a name given with it raises a ValueError) or a pathlib.Path/str path read with load_structure. Signatures (name now defaults to None) and docstrings are updated; the bodies are not implemented yet, so these tests and the existing ones relying on the default name fail until they are. Co-Authored-By: Claude Opus 5.5 --- src/diffpy/cmipdf/basepdfgenerator.py | 35 ++++++++--- src/diffpy/cmipdf/debyepdfgenerator.py | 29 +++++---- tests/test_generators.py | 87 ++++++++++++++++++++++++++ 3 files changed, 130 insertions(+), 21 deletions(-) diff --git a/src/diffpy/cmipdf/basepdfgenerator.py b/src/diffpy/cmipdf/basepdfgenerator.py index be5a223..aa49b3d 100644 --- a/src/diffpy/cmipdf/basepdfgenerator.py +++ b/src/diffpy/cmipdf/basepdfgenerator.py @@ -251,28 +251,43 @@ def get_qmin(self): """ return self._calc.qmin - def set_structure(self, structure, name="phase", periodic=True): + def set_structure(self, structure, name=None, periodic=True): """Set the structure that will be used to calculate the PDF. - This creates a DiffpyStructureParSet, ObjCrystCrystalParSet or - ObjCrystMoleculeParSet that adapts structure to a ParameterSet - interface. - See those classes (located in diffpy.cmistructure) for how they are - used. The resulting ParameterSet will be managed by this generator. + The structure is a structure object, the path to a structure file, + or a ParameterSet that adapts a structure. A structure object is + adapted to a new DiffpyStructureParSet, ObjCrystCrystalParSet or + ObjCrystMoleculeParSet (see diffpy.cmistructure), and a path is + first read into a diffpy.structure Structure. A ParameterSet is + used as it is, with its own name. The ParameterSet is managed by + this generator. + + Giving the same ParameterSet to several generators shares the + structure between them. Giving them the same path does not: each + call reads the file again, so each generator refines its own + structure. Parameters ---------- - structure : Structure or Crystal or Molecule + structure : Structure, Crystal, Molecule, Path, str or ParameterSet The diffpy.structure.Structure, pyobjcryst.crystal.Crystal or - pyobjcryst.molecule.Molecule instance. + pyobjcryst.molecule.Molecule instance, the path to a structure + file that diffpy.structure can read, such as a CIF file, or the + diffpy.cmistructure ParameterSet that adapts a structure. name : str, optional - The name to give to the managed ParameterSet that adapts structure - (default "phase"). + The name to give to the ParameterSet made for a structure object + or a path (default "phase"). It can't be given with a + ParameterSet. periodic : bool, optional The structure should be treated as periodic (default True). Note that some structures do not support periodicity, in which case this will have no effect on the PDF calculation. + + Raises + ------ + ValueError + If a ``name`` is given with a ParameterSet. """ # Create the ParameterSet diff --git a/src/diffpy/cmipdf/debyepdfgenerator.py b/src/diffpy/cmipdf/debyepdfgenerator.py index f6eef39..03fb980 100644 --- a/src/diffpy/cmipdf/debyepdfgenerator.py +++ b/src/diffpy/cmipdf/debyepdfgenerator.py @@ -87,28 +87,35 @@ class DebyePDFGenerator(BasePDFGenerator): See Managed Parameters. """ - def set_structure(self, structure, name="phase", periodic=False): + def set_structure(self, structure, name=None, periodic=False): """Set the structure that will be used to calculate the PDF. - This creates a DiffpyStructureParSet, ObjCrystCrystalParSet or - ObjCrystMoleculeParSet that adapts structure to a ParameterSet - interface. - See those classes (located in diffpy.cmistructure) for how they are - used. The resulting ParameterSet will be managed by this generator. + This is BasePDFGenerator.set_structure with the structure treated + as not periodic by default. The structure is a structure object, + the path to a structure file, or a ParameterSet that adapts a + structure, which is used as it is, with its own name. Parameters ---------- - structure : Structure object - The `diffpy.structure.Structure`, `pyobjcryst.crystal.Crystal` or - `pyobjcryst.molecule.Molecule` instance. + structure : Structure, Crystal, Molecule, Path, str or ParameterSet + The diffpy.structure.Structure, pyobjcryst.crystal.Crystal or + pyobjcryst.molecule.Molecule instance, the path to a structure + file that diffpy.structure can read, such as a CIF file, or the + diffpy.cmistructure ParameterSet that adapts a structure. name : str, optional - A name to give to the managed ParameterSet that adapts structure - (default "phase"). + The name to give to the ParameterSet made for a structure object + or a path (default "phase"). It can't be given with a + ParameterSet. periodic : bool, optional The structure should be treated as periodic (default False). Note that some structures do not support periodicity, in which case this will have no effect on the PDF calculation. + + Raises + ------ + ValueError + If a ``name`` is given with a ParameterSet. """ return BasePDFGenerator.set_structure(self, structure, name, periodic) diff --git a/tests/test_generators.py b/tests/test_generators.py index 5236006..b2fa293 100644 --- a/tests/test_generators.py +++ b/tests/test_generators.py @@ -1,5 +1,8 @@ """Tests for the pdfgenerator module.""" +import re +from pathlib import Path + import numpy as np import pytest @@ -8,6 +11,7 @@ from diffpy.cmistructure.diffpyparset import DiffpyStructureParSet from diffpy.srreal.pdfcalculator import DebyePDFCalculator, PDFCalculator from diffpy.srreal.structureadapter import nosymmetry +from diffpy.structure import load_structure # set_scattering_type stores the radiation type used for the calculation. @@ -104,6 +108,89 @@ def test_set_structure_from_parset( assert actual_state == expected_state +@pytest.mark.parametrize( + "input_generator_class, expected_using_symmetry", + [ + # Test setting the structure from an existing ParameterSet with + # set_structure, as a user does to share one structure between + # generators + # C1: A user gives a ParameterSet named ni to a PDFGenerator. + # Expected: The generator manages that same ParameterSet under its + # own name, ni, and it uses symmetry, as PDFGenerator treats a + # structure as periodic by default. + (PDFGenerator, True), + # C2: A user gives the same kind of ParameterSet to a + # DebyePDFGenerator. + # Expected: The same, except that it does not use symmetry, as + # DebyePDFGenerator treats a structure as not periodic by default. + (DebyePDFGenerator, False), + ], +) +def test_set_structure_with_parameter_set( + ni_pdffit_structure, input_generator_class, expected_using_symmetry +): + parameter_set = DiffpyStructureParSet("ni", ni_pdffit_structure) + generator = input_generator_class() + generator.set_structure(parameter_set) + actual_phase = generator.phase + actual_managed_phase = generator.get("ni") + actual_using_symmetry = parameter_set.using_symmetry() + expected_phase = parameter_set + assert actual_phase == expected_phase + assert actual_managed_phase == expected_phase + assert actual_using_symmetry == expected_using_symmetry + + +def test_set_structure_with_parameter_set_and_name(ni_pdffit_structure): + # Test giving set_structure a name with a ParameterSet, which already + # has its own name + # Expected: A ValueError says the name can't be used and how to name + # the ParameterSet instead. + parameter_set = DiffpyStructureParSet("phase", ni_pdffit_structure) + generator = PDFGenerator() + expected_msg = ( + "set_structure was given the name 'ni' with a ParameterSet, which " + "keeps its own name, 'phase'. Please leave out the name, or give it " + "when you create the ParameterSet." + ) + with pytest.raises(ValueError, match=re.escape(expected_msg)): + generator.set_structure(parameter_set, name="ni") + + +@pytest.mark.parametrize( + "input_generator_class, input_path_type", + [ + # Test setting the structure from the path to a structure file + # C1: A user gives the path to ni.cif to a PDFGenerator as a + # pathlib.Path. + # Expected: The phase is a DiffpyStructureParSet named phase that + # adapts the structure diffpy.structure reads from the file. + (PDFGenerator, Path), + # C2: A user gives the same path as a string. + # Expected: The same as for the Path. + (PDFGenerator, str), + # C3: A user gives the path to a DebyePDFGenerator. + # Expected: The same as for the PDFGenerator. + (DebyePDFGenerator, Path), + ], +) +def test_set_structure_with_structure_file( + datafile, input_generator_class, input_path_type +): + structure_path = input_path_type(datafile("ni.cif")) + generator = input_generator_class() + generator.set_structure(structure_path) + actual_phase_type = type(generator.phase) + actual_phase_name = generator.phase.name + actual_structure = str(generator.phase.structure) + expected_phase_type = DiffpyStructureParSet + expected_phase_name = "phase" + expected_structure = str(load_structure(str(datafile("ni.cif")))) + assert actual_phase_type == expected_phase_type + assert actual_phase_name == expected_phase_name + assert actual_structure == expected_structure + + def test_constrain_as_space_group_accepts_generator(ni_pdffit_structure): # C1: A PDFGenerator holding face-centered cubic Ni is passed to # diffpy.cmistructure's constrain_as_space_group with Fm-3m. From f29b7cd4c59c0496862ac9d83ffc1433f1980b92 Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Mon, 2 Nov 2026 12:28:48 -0500 Subject: [PATCH 2/5] chore: add news item for set_structure taking a ParameterSet or a path Co-Authored-By: Claude Opus 5.5 --- news/set-structure-inputs.rst | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 news/set-structure-inputs.rst diff --git a/news/set-structure-inputs.rst b/news/set-structure-inputs.rst new file mode 100644 index 0000000..a14d252 --- /dev/null +++ b/news/set-structure-inputs.rst @@ -0,0 +1,23 @@ +**Added:** + +* Let ``PDFGenerator.set_structure`` and ``DebyePDFGenerator.set_structure`` take a diffpy.cmistructure ParameterSet, used under its own name, or the path to a structure file, as well as a structure object. + +**Changed:** + +* The ``name`` argument of ``set_structure`` defaults to ``None``, which means "phase" for a structure object or a path. Giving a ``name`` with a ParameterSet raises a ``ValueError``. + +**Deprecated:** + +* + +**Removed:** + +* + +**Fixed:** + +* + +**Security:** + +* From 50df6d8278133ee1fc41c96f524c9138a4edb7b0 Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Mon, 2 Nov 2026 13:09:49 -0500 Subject: [PATCH 3/5] ci: bump docformatter to v1.7.8 in .pre-commit-config.yaml docformatter v1.7.7 depends on untokenize 0.1.1, which fails to build on Python 3.14 (now used by pre-commit.ci). v1.7.8 drops untokenize. Co-Authored-By: Claude Opus 5.5 --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index f189a94..d43370e 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -59,7 +59,7 @@ repos: - "prettier@^3.2.4" # docformatter - PEP 257 compliant docstring formatter - repo: https://github.com/PyCQA/docformatter - rev: v1.7.7 + rev: v1.7.8 hooks: - id: docformatter additional_dependencies: [tomli] From 7436d2588b0dede9001634ee829e3e4afae0c127 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 2 Nov 2026 18:11:06 +0000 Subject: [PATCH 4/5] [pre-commit.ci] auto fixes from pre-commit hooks --- src/diffpy/cmipdf/basepdfgenerator.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/diffpy/cmipdf/basepdfgenerator.py b/src/diffpy/cmipdf/basepdfgenerator.py index aa49b3d..aa5ce1d 100644 --- a/src/diffpy/cmipdf/basepdfgenerator.py +++ b/src/diffpy/cmipdf/basepdfgenerator.py @@ -289,7 +289,6 @@ def set_structure(self, structure, name=None, periodic=True): ValueError If a ``name`` is given with a ParameterSet. """ - # Create the ParameterSet parset = structure_to_parameter_set(name, structure) From 11b5b2e5bf5730ba27bb474fe40c953a3761336b Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Mon, 2 Nov 2026 16:04:16 -0500 Subject: [PATCH 5/5] feat: let set_structure take a ParameterSet or a structure file path BasePDFGenerator.set_structure (and so DebyePDFGenerator's) now uses a diffpy.cmistructure ParameterSet as it is, raising a ValueError if a name is also given, and reads a pathlib.Path or str with diffpy.structure.load_structure. The default name "phase" now applies only to structure objects and paths. test_set_structure_with_parameter_set no longer reads generator.phase, which only exists when the ParameterSet is named "phase"; it checks the ParameterSet managed under its own name instead. Co-Authored-By: Claude Opus 5.5 --- src/diffpy/cmipdf/basepdfgenerator.py | 24 +++++++++++++++++++----- tests/test_generators.py | 6 ++---- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/diffpy/cmipdf/basepdfgenerator.py b/src/diffpy/cmipdf/basepdfgenerator.py index aa49b3d..f81946b 100644 --- a/src/diffpy/cmipdf/basepdfgenerator.py +++ b/src/diffpy/cmipdf/basepdfgenerator.py @@ -21,12 +21,16 @@ __all__ = ["BasePDFGenerator"] +from pathlib import Path + import numpy from diffpy.cmistructure import structure_to_parameter_set +from diffpy.cmistructure.basestructureparset import BaseStructureParSet from diffpy.srfit.exceptions import SrFitError from diffpy.srfit.fitbase import ProfileGenerator from diffpy.srfit.fitbase.parameter import ParameterAdapter +from diffpy.structure import load_structure # FIXME - Parameter creation will have to be smarter once deeper calculator # configuration is enabled. @@ -289,11 +293,21 @@ def set_structure(self, structure, name=None, periodic=True): ValueError If a ``name`` is given with a ParameterSet. """ - - # Create the ParameterSet - parset = structure_to_parameter_set(name, structure) - - # Set the phase + if isinstance(structure, BaseStructureParSet): + if name is not None: + raise ValueError( + f"set_structure was given the name '{name}' with a " + "ParameterSet, which keeps its own name, " + f"'{structure.name}'. Please leave out the name, or give " + "it when you create the ParameterSet." + ) + parset = structure + else: + if isinstance(structure, (str, Path)): + structure = load_structure(str(structure)) + if name is None: + name = "phase" + parset = structure_to_parameter_set(name, structure) self.set_structure_from_parset(parset, periodic) return diff --git a/tests/test_generators.py b/tests/test_generators.py index b2fa293..75222f5 100644 --- a/tests/test_generators.py +++ b/tests/test_generators.py @@ -132,12 +132,10 @@ def test_set_structure_with_parameter_set( parameter_set = DiffpyStructureParSet("ni", ni_pdffit_structure) generator = input_generator_class() generator.set_structure(parameter_set) - actual_phase = generator.phase actual_managed_phase = generator.get("ni") actual_using_symmetry = parameter_set.using_symmetry() - expected_phase = parameter_set - assert actual_phase == expected_phase - assert actual_managed_phase == expected_phase + expected_managed_phase = parameter_set + assert actual_managed_phase == expected_managed_phase assert actual_using_symmetry == expected_using_symmetry