change: let the set_structure method take a path, parameter set, or structure object

.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]

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:**
+
+* <news item>
+
+**Removed:**
+
+* <news item>
+
+**Fixed:**
+
+* <news item>
+
+**Security:**
+
+* <news item>

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.
@@ -251,34 +255,59 @@ 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.
-        """
 
-        # Create the ParameterSet
-        parset = structure_to_parameter_set(name, structure)
-
-        # Set the phase
+        Raises
+        ------
+        ValueError
+            If a ``name`` is given with a ParameterSet.
+        """
+        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
 

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)
 

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,87 @@ 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_managed_phase = generator.get("ni")
+    actual_using_symmetry = parameter_set.using_symmetry()
+    expected_managed_phase = parameter_set
+    assert actual_managed_phase == expected_managed_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.