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] 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:** + +* diff --git a/src/diffpy/cmipdf/basepdfgenerator.py b/src/diffpy/cmipdf/basepdfgenerator.py index be5a223..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. @@ -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 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..75222f5 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,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.