From e8d8ee00c19e676ce5272b3068f77a87cd109125 Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Mon, 26 Oct 2026 17:22:55 -0400 Subject: [PATCH 1/2] ci: remove the diffpy.srfit 3.4.0rc0 bridge and require srfit>=3.4.0 srfit 3.4.0 is on conda-forge, so CI no longer needs to pip install the rc (reverts 000e71b). cmipdf calls methods that srfit 3.3.1 lacks (19 of 49 tests fail with it), so declare the lower bound in conda.txt and pip.txt. Closes #6. Co-Authored-By: Claude Opus 5.5 --- .../matrix-and-codecov-on-merge-to-main.yml | 3 --- .github/workflows/tests-on-pr.yml | 3 --- news/rm-srfit-rc-pin.rst | 23 +++++++++++++++++++ requirements/conda.txt | 2 +- requirements/pip.txt | 2 +- 5 files changed, 25 insertions(+), 8 deletions(-) create mode 100644 news/rm-srfit-rc-pin.rst diff --git a/.github/workflows/matrix-and-codecov-on-merge-to-main.yml b/.github/workflows/matrix-and-codecov-on-merge-to-main.yml index fc33dd8..d9d7476 100644 --- a/.github/workflows/matrix-and-codecov-on-merge-to-main.yml +++ b/.github/workflows/matrix-and-codecov-on-merge-to-main.yml @@ -17,8 +17,5 @@ jobs: project: diffpy.cmipdf c_extension: false headless: false - run: | - set -Eeuo pipefail - python -m pip install --no-deps diffpy.srfit==3.4.0rc0 secrets: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} diff --git a/.github/workflows/tests-on-pr.yml b/.github/workflows/tests-on-pr.yml index e40d41e..3ecbc4c 100644 --- a/.github/workflows/tests-on-pr.yml +++ b/.github/workflows/tests-on-pr.yml @@ -11,8 +11,5 @@ jobs: project: diffpy.cmipdf c_extension: false headless: false - run: | - set -Eeuo pipefail - python -m pip install --no-deps diffpy.srfit==3.4.0rc0 secrets: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} diff --git a/news/rm-srfit-rc-pin.rst b/news/rm-srfit-rc-pin.rst new file mode 100644 index 0000000..4f3ace6 --- /dev/null +++ b/news/rm-srfit-rc-pin.rst @@ -0,0 +1,23 @@ +**Added:** + +* + +**Changed:** + +* + +**Deprecated:** + +* + +**Removed:** + +* + +**Fixed:** + +* Require ``diffpy.srfit`` 3.4.0 or later. Older versions lack the methods diffpy.cmipdf calls, so installing it next to ``diffpy.srfit`` 3.3.1 failed at run time. + +**Security:** + +* diff --git a/requirements/conda.txt b/requirements/conda.txt index 6b1daa8..a5f839f 100644 --- a/requirements/conda.txt +++ b/requirements/conda.txt @@ -1,4 +1,4 @@ numpy diffpy.srreal -diffpy.srfit +diffpy.srfit>=3.4.0 diffpy.structure diff --git a/requirements/pip.txt b/requirements/pip.txt index 6b1daa8..a5f839f 100644 --- a/requirements/pip.txt +++ b/requirements/pip.txt @@ -1,4 +1,4 @@ numpy diffpy.srreal -diffpy.srfit +diffpy.srfit>=3.4.0 diffpy.structure From 3de597a5407e5af055b470c0a6db7e0ac69b618f Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Mon, 26 Oct 2026 17:27:27 -0400 Subject: [PATCH 2/2] fix: build and accept diffpy.cmistructure parameter sets basepdfgenerator.py still built diffpy.srfit.structure parameter sets with struToParameterSet and read .stru and .useSymmetry, so a diffpy.cmistructure parameter set raised AttributeError: stru. Use structure_to_parameter_set, .structure and use_symmetry from diffpy.cmistructure, add it to pip.txt, and pip install it in CI until it is on conda-forge. Tests cover both generators, PDFContribution, and constrain_as_space_group taking a real PDFGenerator. Co-Authored-By: Claude Opus 5.5 --- .../matrix-and-codecov-on-merge-to-main.yml | 3 + .github/workflows/tests-on-pr.yml | 3 + news/use-cmistructure.rst | 23 +++++++ requirements/pip.txt | 1 + src/diffpy/cmipdf/basepdfgenerator.py | 10 +-- src/diffpy/cmipdf/debyepdfgenerator.py | 2 +- tests/test_generators.py | 68 ++++++++++++++++++- tests/test_pdfcontribution.py | 29 ++++++++ 8 files changed, 132 insertions(+), 7 deletions(-) create mode 100644 news/use-cmistructure.rst diff --git a/.github/workflows/matrix-and-codecov-on-merge-to-main.yml b/.github/workflows/matrix-and-codecov-on-merge-to-main.yml index d9d7476..d3b6230 100644 --- a/.github/workflows/matrix-and-codecov-on-merge-to-main.yml +++ b/.github/workflows/matrix-and-codecov-on-merge-to-main.yml @@ -17,5 +17,8 @@ jobs: project: diffpy.cmipdf c_extension: false headless: false + run: | + set -Eeuo pipefail + python -m pip install --no-deps diffpy.cmistructure secrets: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} diff --git a/.github/workflows/tests-on-pr.yml b/.github/workflows/tests-on-pr.yml index 3ecbc4c..62ee924 100644 --- a/.github/workflows/tests-on-pr.yml +++ b/.github/workflows/tests-on-pr.yml @@ -11,5 +11,8 @@ jobs: project: diffpy.cmipdf c_extension: false headless: false + run: | + set -Eeuo pipefail + python -m pip install --no-deps diffpy.cmistructure secrets: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} diff --git a/news/use-cmistructure.rst b/news/use-cmistructure.rst new file mode 100644 index 0000000..6fc7f2e --- /dev/null +++ b/news/use-cmistructure.rst @@ -0,0 +1,23 @@ +**Added:** + +* ``diffpy.cmistructure`` as a dependency. + +**Changed:** + +* + +**Deprecated:** + +* + +**Removed:** + +* + +**Fixed:** + +* Build ``diffpy.cmistructure`` parameter sets in ``set_structure`` and ``add_structure``, and accept them in ``set_structure_from_parset`` and ``add_structure_from_parset``. These used ``diffpy.srfit.structure`` and raised ``AttributeError: stru`` for a ``diffpy.cmistructure`` parameter set. + +**Security:** + +* diff --git a/requirements/pip.txt b/requirements/pip.txt index a5f839f..d65e8de 100644 --- a/requirements/pip.txt +++ b/requirements/pip.txt @@ -2,3 +2,4 @@ numpy diffpy.srreal diffpy.srfit>=3.4.0 diffpy.structure +diffpy.cmistructure diff --git a/src/diffpy/cmipdf/basepdfgenerator.py b/src/diffpy/cmipdf/basepdfgenerator.py index b94827a..be5a223 100644 --- a/src/diffpy/cmipdf/basepdfgenerator.py +++ b/src/diffpy/cmipdf/basepdfgenerator.py @@ -23,10 +23,10 @@ import numpy +from diffpy.cmistructure import structure_to_parameter_set from diffpy.srfit.exceptions import SrFitError from diffpy.srfit.fitbase import ProfileGenerator from diffpy.srfit.fitbase.parameter import ParameterAdapter -from diffpy.srfit.structure import struToParameterSet # FIXME - Parameter creation will have to be smarter once deeper calculator # configuration is enabled. @@ -257,7 +257,7 @@ def set_structure(self, structure, name="phase", periodic=True): This creates a DiffpyStructureParSet, ObjCrystCrystalParSet or ObjCrystMoleculeParSet that adapts structure to a ParameterSet interface. - See those classes (located in diffpy.srfit.structure) for how they are + See those classes (located in diffpy.cmistructure) for how they are used. The resulting ParameterSet will be managed by this generator. Parameters @@ -276,7 +276,7 @@ def set_structure(self, structure, name="phase", periodic=True): """ # Create the ParameterSet - parset = struToParameterSet(name, structure) + parset = structure_to_parameter_set(name, structure) # Set the phase self.set_structure_from_parset(parset, periodic) @@ -304,13 +304,13 @@ def set_structure_from_parset(self, parset, periodic=True): """ # Store the ParameterSet for easy access self._phase = parset - self.structure = self._phase.stru + self.structure = self._phase.structure # Put this ParameterSet in the ProfileGenerator. self.add_parameter_set(parset) # Set periodicity - self._phase.useSymmetry(periodic) + self._phase.use_symmetry(periodic) return def _prepare(self, r): diff --git a/src/diffpy/cmipdf/debyepdfgenerator.py b/src/diffpy/cmipdf/debyepdfgenerator.py index 1825b5d..f6eef39 100644 --- a/src/diffpy/cmipdf/debyepdfgenerator.py +++ b/src/diffpy/cmipdf/debyepdfgenerator.py @@ -93,7 +93,7 @@ def set_structure(self, structure, name="phase", periodic=False): This creates a DiffpyStructureParSet, ObjCrystCrystalParSet or ObjCrystMoleculeParSet that adapts structure to a ParameterSet interface. - See those classes (located in diffpy.srfit.structure) for how they are + See those classes (located in diffpy.cmistructure) for how they are used. The resulting ParameterSet will be managed by this generator. Parameters diff --git a/tests/test_generators.py b/tests/test_generators.py index 5cbff63..d7bacb2 100644 --- a/tests/test_generators.py +++ b/tests/test_generators.py @@ -3,7 +3,9 @@ import numpy as np import pytest -from diffpy.cmipdf import PDFGenerator +from diffpy.cmipdf import DebyePDFGenerator, PDFGenerator +from diffpy.cmistructure import constrain_as_space_group +from diffpy.cmistructure.diffpyparset import DiffpyStructureParSet from diffpy.srreal.pdfcalculator import PDFCalculator @@ -51,6 +53,70 @@ def test_set_qmin(): assert actual_qmins == expected_qmins +# set_structure adapts the structure with a diffpy.cmistructure parameter +# set, which the generator then manages as its phase. +@pytest.mark.parametrize( + "input_generator_class", + [ + # C1: A diffpy.structure Structure is set on a generator, + # 1. PDFGenerator, 2. DebyePDFGenerator. + # Expected: The phase is a diffpy.cmistructure DiffpyStructureParSet + # that adapts the structure. + PDFGenerator, + DebyePDFGenerator, + ], +) +def test_set_structure(ni_pdffit_structure, input_generator_class): + generator = input_generator_class() + generator.set_structure(ni_pdffit_structure) + actual_phase = (type(generator.phase), generator.phase.structure) + expected_phase = (DiffpyStructureParSet, ni_pdffit_structure) + assert actual_phase == expected_phase + + +# set_structure_from_parset manages the given diffpy.cmistructure parameter +# set as the phase, and periodic sets whether it uses symmetry. +@pytest.mark.parametrize( + "input_periodic, expected_using_symmetry", + [ + # C1: The parameter set is set as periodic. + # Expected: The generator manages that parameter set, its structure + # is the adapted structure, and the parameter set uses symmetry. + (True, True), + # C2: The parameter set is set as not periodic. + # Expected: The same, except that it does not use symmetry. + (False, False), + ], +) +def test_set_structure_from_parset( + ni_pdffit_structure, input_periodic, expected_using_symmetry +): + parameter_set = DiffpyStructureParSet("phase", ni_pdffit_structure) + generator = PDFGenerator() + generator.set_structure_from_parset(parameter_set, periodic=input_periodic) + actual_state = ( + generator.phase is parameter_set, + generator.structure is ni_pdffit_structure, + parameter_set.using_symmetry(), + ) + expected_state = (True, True, expected_using_symmetry) + assert actual_state == expected_state + + +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. + # Expected: Only the lattice length a of its phase is left free. + generator = PDFGenerator() + generator.set_structure(ni_pdffit_structure) + space_group_parameters = constrain_as_space_group(generator, "Fm-3m") + actual_free_lattice_names = [ + parameter.name for parameter in space_group_parameters.latpars + ] + expected_free_lattice_names = ["a"] + assert actual_free_lattice_names == expected_free_lattice_names + + def test_parameter_names(ni_pdffit_structure): # C1: A structure is set on the generator. # Expected: The generator holds one Parameter for each calculator diff --git a/tests/test_pdfcontribution.py b/tests/test_pdfcontribution.py index 466a0d1..0ed3631 100644 --- a/tests/test_pdfcontribution.py +++ b/tests/test_pdfcontribution.py @@ -8,6 +8,7 @@ import pytest from diffpy.cmipdf import PDFContribution +from diffpy.cmistructure.diffpyparset import DiffpyStructureParSet from diffpy.srfit.exceptions import SrFitError from diffpy.structure import Structure, load_structure @@ -126,3 +127,31 @@ def test_pickling(datafile): actual_residual = unpickled_contribution.residual() expected_residual = _make_ni_contribution(datafile, uiso=0.004).residual() assert np.allclose(actual_residual, expected_residual) + + +def test_add_structure(datafile): + # C1: A diffpy.structure Structure is added to the contribution. + # Expected: The new generator's phase is a diffpy.cmistructure + # DiffpyStructureParSet that adapts the structure. + contribution = PDFContribution("pdf") + ni = load_structure(str(datafile("ni.cif"))) + contribution.add_structure(ni, name="ni") + actual_phase = ( + type(contribution.ni.phase), + contribution.ni.phase.structure, + ) + expected_phase = (DiffpyStructureParSet, ni) + assert actual_phase == expected_phase + + +def test_add_structure_from_parset(datafile): + # C1: A diffpy.cmistructure DiffpyStructureParSet is added to the + # contribution. + # Expected: The new generator manages that parameter set as its phase. + contribution = PDFContribution("pdf") + ni = load_structure(str(datafile("ni.cif"))) + parameter_set = DiffpyStructureParSet("phase", ni) + contribution.add_structure_from_parset(parameter_set, name="ni") + actual_phase = contribution.ni.phase + expected_phase = parameter_set + assert actual_phase is expected_phase