From 0cb1c926d9047925cb595d5a8d28f8ba1b9baad4 Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Fri, 14 Aug 2026 11:37:55 -0600 Subject: [PATCH 1/7] refactor: place pdf testing into their own separate files --- tests/test_pdf.py | 517 ---------------------------------- tests/test_pdfcontribution.py | 150 ++++++++++ tests/test_pdfgenerator.py | 92 ++++++ tests/test_pdfparser.py | 201 +++++++++++++ tests/test_recipeorganizer.py | 111 ++++++++ 5 files changed, 554 insertions(+), 517 deletions(-) delete mode 100644 tests/test_pdf.py create mode 100644 tests/test_pdfcontribution.py create mode 100644 tests/test_pdfgenerator.py create mode 100644 tests/test_pdfparser.py diff --git a/tests/test_pdf.py b/tests/test_pdf.py deleted file mode 100644 index 4a6b1136..00000000 --- a/tests/test_pdf.py +++ /dev/null @@ -1,517 +0,0 @@ -#!/usr/bin/env python -############################################################################## -# -# diffpy.srfit by DANSE Diffraction group -# Simon J. L. Billinge -# (c) 2010 The Trustees of Columbia University -# in the City of New York. All rights reserved. -# -# File coded by: Pavol Juhas -# -# See AUTHORS.txt for a list of people who contributed. -# See LICENSE_DANSE.txt for license information. -# -############################################################################## -"""Tests for pdf package.""" - -import io -import pickle -import unittest -from itertools import chain - -import numpy -import pytest - -from diffpy.srfit.exceptions import SrFitError -from diffpy.srfit.fitbase.parameter import Parameter -from diffpy.srfit.fitbase.recipeorganizer import RecipeContainer -from diffpy.srfit.pdf import PDFContribution, PDFGenerator, PDFParser - -# ---------------------------------------------------------------------------- - - -def approx_or_none(expected_values): - """Wrap expected_values in pytest.approx, unless it is None.""" - if expected_values is None: - return None - return pytest.approx(expected_values) - - -@pytest.mark.parametrize( - "input_filename, expected_x, expected_y, expected_dy", - [ - # C1: A neutron PDF written by PDFgetN, which has no dx or dy - # columns. - # Expected: x and y are read correctly, and dx and dy are None. - ( - "ni-q27r100-neutron.gr", - numpy.linspace(0.01, 100, 10000), - [ - 1.144, - 2.258, - 3.312, - 4.279, - 5.135, - 5.862, - 6.445, - 6.875, - 7.150, - 7.272, - ], - None, - ), - # C2: An x-ray PDF written by PDFgetX2, which has a dy column - # and a negative dx column. - # Expected: x, y, and dy are read correctly, and the invalid - # negative dx column is dropped. - ( - "si-q27r60-xray.gr", - numpy.linspace(0.01, 60, 5999, endpoint=False), - [ - 0.1105784, - 0.2199684, - 0.3270088, - 0.4305913, - 0.5296853, - 0.6233606, - 0.7108060, - 0.7913456, - 0.8644501, - 0.9297440, - ], - [ - 0.001802192, - 0.003521449, - 0.005079115, - 0.006404892, - 0.007440527, - 0.008142955, - 0.008486813, - 0.008466340, - 0.008096858, - 0.007416456, - ], - ), - ], -) -def test_pdfparser_data( - datafile, as_list, input_filename, expected_x, expected_y, expected_dy -): - """PDFParser reads the x, y, and dy arrays correctly, and always - drops the invalid dx column.""" - parser = PDFParser() - parser.parse_file(datafile(input_filename)) - - actual_x, actual_y, actual_dx, actual_dy = parser.get_data() - actual_dy = as_list(actual_dy) - if actual_dy is not None: - # Compare only the first 10 values - actual_dy = actual_dy[:10] - assert actual_dx is None - assert actual_x.tolist() == pytest.approx(expected_x.tolist()) - assert actual_y[:10].tolist() == pytest.approx(expected_y) - assert actual_dy == approx_or_none(expected_dy) - - -# PDFParser inherits ProfileParser's hooks unchanged: PDFgetX and -# PDFgetN headers are already plain name = value pairs, including -# stype = X or stype = N for the scattering type. The metadata below -# reaches PDFGenerator, which uses stype, qmin and qmax to set the -# scattering type and the Q range, so losing a key silently changes a -# refinement. -@pytest.mark.parametrize( - "input_filename, expected_metadata", - [ - # C1: An x-ray PDF written by PDFgetX2. - # Expected: The header yields the x-ray scattering type, - # the Q range and the rest of the diffpy.pdfgetx config. - ( - "si-q27r60-xray.gr", - { - "version": "diffpy.pdfgetx-2.4.0", - "dataformat": "QA", - "outputtype": "gr", - "stype": "X", - "composition": "Si", - "bgscale": 1.0, - "rpoly": 0.9, - "qmaxinst": 29.0, - "qmin": 0.01, - "qmax": 27.0, - "rmin": 0.0, - "rmax": 60.0, - "rstep": 0.01, - "temperature": 300.0, - "bank": 0, - "nbanks": 1, - }, - ), - # C2: A neutron PDF written by PDFgetN. - # Expected: The header yields the neutron scattering type, - # the Q range and the rest of the xPDFsuite config. - ( - "ni-q27r100-neutron.gr", - { - "wavelength": 1.333, - "dataformat": "QA", - "inputfile": "npdf_03315.chi", - "backgroundfile": "npdf_03001.chi", - "stype": "N", - "bgscale": 1.0, - "composition": "Ni", - "outputtype": "gr", - "qmaxinst": 27.0, - "qmin": 0.87, - "qmax": 27.0, - "temperature": 300.0, - "rmax": 100.0, - "rmin": 0.0, - "rstep": 0.01, - "rpoly": 0.9, - "inputdir": "/data/npdf/chi", - "savedir": "/data/npdf/gr", - "bank": 0, - "nbanks": 1, - }, - ), - ], -) -def test_pdfparser_metadata(datafile, input_filename, expected_metadata): - """PDF specific metadata survives the load_data based parse_file.""" - parser = PDFParser() - parser.parse_file(datafile(input_filename)) - actual_metadata = parser.get_metadata() - # add the filename key to the expected metadata for comparison - expected_metadata["filename"] = str(datafile(input_filename)) - assert actual_metadata == expected_metadata - - -def test_pdfparser_deprecated_parseFile(datafile): - """The deprecated parseFile warns and delegates to parse_file.""" - input_filename = datafile("si-q27r60-xray.gr") - expected_parser = PDFParser() - expected_parser.parse_file(input_filename) - actual_parser = PDFParser() - with pytest.warns(DeprecationWarning): - actual_parser.parseFile(input_filename) - - actual_metadata = actual_parser.get_metadata() - expected_metadata = expected_parser.get_metadata() - assert actual_metadata == expected_metadata - - actual_x, actual_y, actual_dx, actual_dy = actual_parser.get_data() - expected_x, expected_y, expected_dx, expected_dy = ( - expected_parser.get_data() - ) - assert actual_x.tolist() == expected_x.tolist() - assert actual_y.tolist() == expected_y.tolist() - assert actual_dx == expected_dx - assert actual_dy.tolist() == expected_dy.tolist() - - -def test_pdfcontribution_loadData(datafile): - """LoadData passes the PDF metadata on to the built-in profile.""" - contribution = PDFContribution("pdf") - contribution.loadData(datafile("si-q27r60-xray.gr")) - - expected_metadata = { - "version": "diffpy.pdfgetx-2.4.0", - "dataformat": "QA", - "outputtype": "gr", - "stype": "X", - "composition": "Si", - "bgscale": 1.0, - "rpoly": 0.9, - "qmaxinst": 29.0, - "qmin": 0.01, - "qmax": 27.0, - "rmin": 0.0, - "rmax": 60.0, - "rstep": 0.01, - "temperature": 300.0, - "filename": str(datafile("si-q27r60-xray.gr")), - "bank": 0, - "nbanks": 1, - } - actual_metadata = contribution.profile.meta - assert actual_metadata == expected_metadata - actual_point_count = len(contribution.profile.xobs) - expected_point_count = 5999 - assert actual_point_count == expected_point_count - - -def testGenerator(diffpy_srreal_available, datafile): - if not diffpy_srreal_available: - pytest.skip("diffpy.srreal package not available") - - from diffpy.srreal.pdfcalculator import PDFCalculator - from diffpy.structure import PDFFitStructure - - qmax = 27.0 - gen = PDFGenerator() - gen.setScatteringType("N") - assert "N" == gen.getScatteringType() - gen.setQmax(qmax) - assert qmax == pytest.approx(gen.getQmax()) - - stru = PDFFitStructure() - ciffile = datafile("ni.cif") - cif_path = str(ciffile) - stru.read(cif_path) - for i in range(4): - stru[i].Bisoequiv = 1 - gen.setStructure(stru) - - calc = gen._calc - # Test parameters - for par in gen.iterPars(recurse=False): - pname = par.name - defval = calc._getDoubleAttr(pname) - assert defval == par.getValue() - # Test setting values - par.set_value(1.0) - assert 1.0 == par.getValue() - par.set_value(defval) - assert defval == par.getValue() - - r = numpy.arange(0, 10, 0.1) - y = gen(r) - - # Now create a reference PDF. Since the calculator is testing its - # output, we just have to make sure we can calculate from the - # PDFGenerator interface. - - calc = PDFCalculator() - calc.rstep = r[1] - r[0] - calc.rmin = r[0] - calc.rmax = r[-1] + 0.5 * calc.rstep - calc.qmax = qmax - calc.setScatteringFactorTableByType("N") - calc.eval(stru) - yref = calc.pdf - - diff = y - yref - res = numpy.dot(diff, diff) - assert 0 == pytest.approx(res) - return - - -def test_setQmin(diffpy_srreal_available): - """Verify qmin is propagated to the calculator object.""" - if not diffpy_srreal_available: - pytest.skip("diffpy.srreal package not available") - - gen = PDFGenerator() - assert 0 == gen.getQmin() - assert 0 == gen._calc.qmin - gen.setQmin(0.93) - assert 0.93 == gen.getQmin() - assert 0.93 == gen._calc.qmin - return - - -def test_setQmax(diffpy_srreal_available): - """Check PDFContribution.setQmax()""" - from diffpy.structure import Structure - - if not diffpy_srreal_available: - pytest.skip("diffpy.srreal package not available") - - pc = PDFContribution("pdf") - pc.setQmax(21) - pc.addStructure("empty", Structure()) - assert 21 == pc.empty.getQmax() - pc.setQmax(22) - assert 22 == pc.getQmax() - assert 22 == pc.empty.getQmax() - return - - -def test_getQmax(diffpy_srreal_available): - """Check PDFContribution.getQmax()""" - from diffpy.structure import Structure - - if not diffpy_srreal_available: - pytest.skip("diffpy.srreal package not available") - - # cover all code branches in PDFContribution._get_meta_value - # (1) contribution metadata - pc1 = PDFContribution("pdf") - assert pc1.getQmax() is None - pc1.setQmax(17) - assert 17 == pc1.getQmax() - # (2) contribution metadata - pc2 = PDFContribution("pdf") - pc2.addStructure("empty", Structure()) - pc2.empty.setQmax(18) - assert 18 == pc2.getQmax() - # (3) profile metadata - pc3 = PDFContribution("pdf") - pc3.profile.meta["qmax"] = 19 - assert 19 == pc3.getQmax() - return - - -def test_savetxt(diffpy_srreal_available, datafile): - "check PDFContribution.savetxt()" - from diffpy.structure import Structure - - if not diffpy_srreal_available: - pytest.skip("diffpy.srreal package not available") - - pc = PDFContribution("pdf") - pc.loadData(datafile("si-q27r60-xray.gr")) - pc.setCalculationRange(0, 10) - pc.addStructure("empty", Structure()) - fp = io.BytesIO() - with pytest.raises(SrFitError): - pc.savetxt(fp) - pc.evaluate() - pc.savetxt(fp) - txt = fp.getvalue().decode() - nlines = len(txt.strip().split("\n")) - assert 1001 == nlines - return - - -def test_pickling(diffpy_srreal_available, datafile): - "validate PDFContribution.residual() after pickling." - from diffpy.structure import loadStructure - - if not diffpy_srreal_available: - pytest.skip("diffpy.srreal package not available") - - pc = PDFContribution("pdf") - pc.loadData(datafile("ni-q27r100-neutron.gr")) - ciffile = datafile("ni.cif") - cif_path = str(ciffile) - ni = loadStructure(cif_path) - ni.Uisoequiv = 0.003 - pc.addStructure("ni", ni) - pc.setCalculationRange(0, 10) - pc2 = pickle.loads(pickle.dumps(pc)) - res0 = pc.residual() - assert numpy.array_equal(res0, pc2.residual()) - for p in chain( - pc.iterate_over_parameters("Uiso"), pc2.iterate_over_parameters("Uiso") - ): - p.value = 0.004 - res1 = pc.residual() - assert not numpy.allclose(res0, res1) - assert numpy.array_equal(res1, pc2.residual()) - return - - -if __name__ == "__main__": - unittest.main() - - -def _make_iterpars_tree(): - """Build a small hierarchy for iterPars tests.""" - root = RecipeContainer("root") - root._containers = {} - root._manage(root._containers) - - root_biso = Parameter("Biso", 10) - root._add_object(root_biso, root._parameters) - - ni0 = RecipeContainer("Ni0") - ni0_biso = Parameter("Biso", 20) - ni0_uiso = Parameter("Uiso", 30) - ni0._add_object(ni0_biso, ni0._parameters) - ni0._add_object(ni0_uiso, ni0._parameters) - - ni1 = RecipeContainer("Ni1") - ni1_biso = Parameter("Biso", 40) - ni1._add_object(ni1_biso, ni1._parameters) - - o0 = RecipeContainer("O0") - o0_biso = Parameter("Biso", 50) - o0._add_object(o0_biso, o0._parameters) - - root._add_object(ni0, root._containers) - root._add_object(ni1, root._containers) - root._add_object(o0, root._containers) - - return { - "root": root, - "root_biso": root_biso, - "ni0": ni0, - "ni0_biso": ni0_biso, - "ni0_uiso": ni0_uiso, - "ni1": ni1, - "ni1_biso": ni1_biso, - "o0": o0, - "o0_biso": o0_biso, - } - - -@pytest.mark.parametrize( - ("pattern", "kwargs", "expected_values"), - [ - # C1: Match leaf parameter names without fullnames. - # Expected: all Biso parameters in the hierarchy are returned. - (r"^Biso$", {}, [10, 20, 40, 50]), - # C2: Match hierarchical names without fullnames. - # Expected: no leaf names match the hierarchical pattern. - (r"^Ni\d+\.Biso$", {}, []), - # C3: Match hierarchical names with fullnames enabled. - # Expected: matching Ni Biso parameters are returned. - (r"^Ni\d+\.Biso$", {"fullnames": True}, [20, 40]), - # C4: Match one hierarchical Uiso name. - # Expected: only Ni0.Uiso is returned. - (r"^Ni0\.Uiso$", {"fullnames": True}, [30]), - # C5: Match one hierarchical Biso name outside Ni containers. - # Expected: only O0.Biso is returned. - (r"^O0\.Biso$", {"fullnames": True}, [50]), - # C6: Disable recursion while matching child fullnames. - # Expected: no child parameters are returned. - (r"^Ni\d+\.Biso$", {"fullnames": True, "recurse": False}, []), - # C7: Disable recursion while matching root fullname. - # Expected: only the root-level Biso parameter is returned. - (r"^Biso$", {"fullnames": True, "recurse": False}, [10]), - ], -) -def test_iterpars_fullname_matching(pattern, kwargs, expected_values): - """Verify leaf-name and fullname matching in - iterate_over_parameters.""" - objs = _make_iterpars_tree() - root = objs["root"] - - actual_values = [ - parameter.value - for parameter in root.iterate_over_parameters(pattern, **kwargs) - ] - - assert actual_values == expected_values - - -@pytest.mark.parametrize( - ("pattern", "expected_name"), - [ - # C1: Match Biso relative to the called Ni0 container. - # Expected: Ni0.Biso is returned without the Ni0 prefix. - (r"^Biso$", ["Biso"]), - # C2: Match Uiso relative to the called Ni0 container. - # Expected: Ni0.Uiso is returned without the Ni0 prefix. - (r"^Uiso$", ["Uiso"]), - # C3: Match with the parent container prefix from inside Ni0. - # Expected: no parameter is returned because fullnames are relative - # to the container on which iterate_over_parameters is called. - (r"^Ni0\.Biso$", []), - ], -) -def test_iterpars_fullnames_are_relative_to_called_container( - pattern, - expected_name, -): - """Verify fullname matching is relative to the called container.""" - objs = _make_iterpars_tree() - ni0 = objs["ni0"] - - actual_name = [ - parameter.name - for parameter in ni0.iterate_over_parameters(pattern, fullnames=True) - ] - - assert actual_name == expected_name diff --git a/tests/test_pdfcontribution.py b/tests/test_pdfcontribution.py new file mode 100644 index 00000000..69642404 --- /dev/null +++ b/tests/test_pdfcontribution.py @@ -0,0 +1,150 @@ +#!/usr/bin/env python +############################################################################## +# +# diffpy.srfit by DANSE Diffraction group +# Simon J. L. Billinge +# (c) 2010 The Trustees of Columbia University +# in the City of New York. All rights reserved. +# +# File coded by: Pavol Juhas +# +# See AUTHORS.txt for a list of people who contributed. +# See LICENSE_DANSE.txt for license information. +# +############################################################################## +"""Tests for pdf.pdfcontribution module.""" + +import io +import pickle +from itertools import chain + +import numpy +import pytest + +from diffpy.srfit.exceptions import SrFitError +from diffpy.srfit.pdf import PDFContribution + +# ---------------------------------------------------------------------------- + + +def test_pdfcontribution_loadData(datafile): + """LoadData passes the PDF metadata on to the built-in profile.""" + contribution = PDFContribution("pdf") + contribution.loadData(datafile("si-q27r60-xray.gr")) + + expected_metadata = { + "version": "diffpy.pdfgetx-2.4.0", + "dataformat": "QA", + "outputtype": "gr", + "stype": "X", + "composition": "Si", + "bgscale": 1.0, + "rpoly": 0.9, + "qmaxinst": 29.0, + "qmin": 0.01, + "qmax": 27.0, + "rmin": 0.0, + "rmax": 60.0, + "rstep": 0.01, + "temperature": 300.0, + "filename": str(datafile("si-q27r60-xray.gr")), + "bank": 0, + "nbanks": 1, + } + actual_metadata = contribution.profile.meta + assert actual_metadata == expected_metadata + actual_point_count = len(contribution.profile.xobs) + expected_point_count = 5999 + assert actual_point_count == expected_point_count + + +def test_setQmax(diffpy_srreal_available): + """Check PDFContribution.setQmax()""" + from diffpy.structure import Structure + + if not diffpy_srreal_available: + pytest.skip("diffpy.srreal package not available") + + pc = PDFContribution("pdf") + pc.setQmax(21) + pc.addStructure("empty", Structure()) + assert 21 == pc.empty.getQmax() + pc.setQmax(22) + assert 22 == pc.getQmax() + assert 22 == pc.empty.getQmax() + return + + +def test_getQmax(diffpy_srreal_available): + """Check PDFContribution.getQmax()""" + from diffpy.structure import Structure + + if not diffpy_srreal_available: + pytest.skip("diffpy.srreal package not available") + + # cover all code branches in PDFContribution._get_meta_value + # (1) contribution metadata + pc1 = PDFContribution("pdf") + assert pc1.getQmax() is None + pc1.setQmax(17) + assert 17 == pc1.getQmax() + # (2) contribution metadata + pc2 = PDFContribution("pdf") + pc2.addStructure("empty", Structure()) + pc2.empty.setQmax(18) + assert 18 == pc2.getQmax() + # (3) profile metadata + pc3 = PDFContribution("pdf") + pc3.profile.meta["qmax"] = 19 + assert 19 == pc3.getQmax() + return + + +def test_savetxt(diffpy_srreal_available, datafile): + "check PDFContribution.savetxt()" + from diffpy.structure import Structure + + if not diffpy_srreal_available: + pytest.skip("diffpy.srreal package not available") + + pc = PDFContribution("pdf") + pc.loadData(datafile("si-q27r60-xray.gr")) + pc.setCalculationRange(0, 10) + pc.addStructure("empty", Structure()) + fp = io.BytesIO() + with pytest.raises(SrFitError): + pc.savetxt(fp) + pc.evaluate() + pc.savetxt(fp) + txt = fp.getvalue().decode() + nlines = len(txt.strip().split("\n")) + assert 1001 == nlines + return + + +def test_pickling(diffpy_srreal_available, datafile): + "validate PDFContribution.residual() after pickling." + from diffpy.structure import loadStructure + + if not diffpy_srreal_available: + pytest.skip("diffpy.srreal package not available") + + pc = PDFContribution("pdf") + pc.loadData(datafile("ni-q27r100-neutron.gr")) + ciffile = datafile("ni.cif") + cif_path = str(ciffile) + ni = loadStructure(cif_path) + ni.Uisoequiv = 0.003 + pc.addStructure("ni", ni) + pc.setCalculationRange(0, 10) + pc2 = pickle.loads(pickle.dumps(pc)) + res0 = pc.residual() + assert numpy.array_equal(res0, pc2.residual()) + for p in chain( + pc.iterate_over_parameters("Uiso"), pc2.iterate_over_parameters("Uiso") + ): + p.value = 0.004 + res1 = pc.residual() + assert not numpy.allclose(res0, res1) + assert numpy.array_equal(res1, pc2.residual()) + return diff --git a/tests/test_pdfgenerator.py b/tests/test_pdfgenerator.py new file mode 100644 index 00000000..0c56933a --- /dev/null +++ b/tests/test_pdfgenerator.py @@ -0,0 +1,92 @@ +#!/usr/bin/env python +############################################################################## +# +# diffpy.srfit by DANSE Diffraction group +# Simon J. L. Billinge +# (c) 2010 The Trustees of Columbia University +# in the City of New York. All rights reserved. +# +# File coded by: Pavol Juhas +# +# See AUTHORS.txt for a list of people who contributed. +# See LICENSE_DANSE.txt for license information. +# +############################################################################## +"""Tests for pdf.pdfgenerator module.""" + +import numpy +import pytest + +from diffpy.srfit.pdf import PDFGenerator + +# ---------------------------------------------------------------------------- + + +def testGenerator(diffpy_srreal_available, datafile): + if not diffpy_srreal_available: + pytest.skip("diffpy.srreal package not available") + + from diffpy.srreal.pdfcalculator import PDFCalculator + from diffpy.structure import PDFFitStructure + + qmax = 27.0 + gen = PDFGenerator() + gen.setScatteringType("N") + assert "N" == gen.getScatteringType() + gen.setQmax(qmax) + assert qmax == pytest.approx(gen.getQmax()) + + stru = PDFFitStructure() + ciffile = datafile("ni.cif") + cif_path = str(ciffile) + stru.read(cif_path) + for i in range(4): + stru[i].Bisoequiv = 1 + gen.setStructure(stru) + + calc = gen._calc + # Test parameters + for par in gen.iterPars(recurse=False): + pname = par.name + defval = calc._getDoubleAttr(pname) + assert defval == par.getValue() + # Test setting values + par.set_value(1.0) + assert 1.0 == par.getValue() + par.set_value(defval) + assert defval == par.getValue() + + r = numpy.arange(0, 10, 0.1) + y = gen(r) + + # Now create a reference PDF. Since the calculator is testing its + # output, we just have to make sure we can calculate from the + # PDFGenerator interface. + + calc = PDFCalculator() + calc.rstep = r[1] - r[0] + calc.rmin = r[0] + calc.rmax = r[-1] + 0.5 * calc.rstep + calc.qmax = qmax + calc.setScatteringFactorTableByType("N") + calc.eval(stru) + yref = calc.pdf + + diff = y - yref + res = numpy.dot(diff, diff) + assert 0 == pytest.approx(res) + return + + +def test_setQmin(diffpy_srreal_available): + """Verify qmin is propagated to the calculator object.""" + if not diffpy_srreal_available: + pytest.skip("diffpy.srreal package not available") + + gen = PDFGenerator() + assert 0 == gen.getQmin() + assert 0 == gen._calc.qmin + gen.setQmin(0.93) + assert 0.93 == gen.getQmin() + assert 0.93 == gen._calc.qmin + return diff --git a/tests/test_pdfparser.py b/tests/test_pdfparser.py new file mode 100644 index 00000000..65bc1114 --- /dev/null +++ b/tests/test_pdfparser.py @@ -0,0 +1,201 @@ +#!/usr/bin/env python +############################################################################## +# +# diffpy.srfit by DANSE Diffraction group +# Simon J. L. Billinge +# (c) 2010 The Trustees of Columbia University +# in the City of New York. All rights reserved. +# +# File coded by: Pavol Juhas +# +# See AUTHORS.txt for a list of people who contributed. +# See LICENSE_DANSE.txt for license information. +# +############################################################################## +"""Tests for pdf.pdfparser module.""" + +import numpy +import pytest + +from diffpy.srfit.pdf import PDFParser + +# ---------------------------------------------------------------------------- + + +def approx_or_none(expected_values): + """Wrap expected_values in pytest.approx, unless it is None.""" + if expected_values is None: + return None + return pytest.approx(expected_values) + + +@pytest.mark.parametrize( + "input_filename, expected_x, expected_y, expected_dy", + [ + # C1: A neutron PDF written by PDFgetN, which has no dx or dy + # columns. + # Expected: x and y are read correctly, and dx and dy are None. + ( + "ni-q27r100-neutron.gr", + numpy.linspace(0.01, 100, 10000), + [ + 1.144, + 2.258, + 3.312, + 4.279, + 5.135, + 5.862, + 6.445, + 6.875, + 7.150, + 7.272, + ], + None, + ), + # C2: An x-ray PDF written by PDFgetX2, which has a dy column + # and a negative dx column. + # Expected: x, y, and dy are read correctly, and the invalid + # negative dx column is dropped. + ( + "si-q27r60-xray.gr", + numpy.linspace(0.01, 60, 5999, endpoint=False), + [ + 0.1105784, + 0.2199684, + 0.3270088, + 0.4305913, + 0.5296853, + 0.6233606, + 0.7108060, + 0.7913456, + 0.8644501, + 0.9297440, + ], + [ + 0.001802192, + 0.003521449, + 0.005079115, + 0.006404892, + 0.007440527, + 0.008142955, + 0.008486813, + 0.008466340, + 0.008096858, + 0.007416456, + ], + ), + ], +) +def test_pdfparser_data( + datafile, as_list, input_filename, expected_x, expected_y, expected_dy +): + """PDFParser reads the x, y, and dy arrays correctly, and always + drops the invalid dx column.""" + parser = PDFParser() + parser.parse_file(datafile(input_filename)) + + actual_x, actual_y, actual_dx, actual_dy = parser.get_data() + actual_dy = as_list(actual_dy) + if actual_dy is not None: + # Compare only the first 10 values + actual_dy = actual_dy[:10] + assert actual_dx is None + assert actual_x.tolist() == pytest.approx(expected_x.tolist()) + assert actual_y[:10].tolist() == pytest.approx(expected_y) + assert actual_dy == approx_or_none(expected_dy) + + +# PDFParser inherits ProfileParser's hooks unchanged: PDFgetX and +# PDFgetN headers are already plain name = value pairs, including +# stype = X or stype = N for the scattering type. The metadata below +# reaches PDFGenerator, which uses stype, qmin and qmax to set the +# scattering type and the Q range, so losing a key silently changes a +# refinement. +@pytest.mark.parametrize( + "input_filename, expected_metadata", + [ + # C1: An x-ray PDF written by PDFgetX2. + # Expected: The header yields the x-ray scattering type, + # the Q range and the rest of the diffpy.pdfgetx config. + ( + "si-q27r60-xray.gr", + { + "version": "diffpy.pdfgetx-2.4.0", + "dataformat": "QA", + "outputtype": "gr", + "stype": "X", + "composition": "Si", + "bgscale": 1.0, + "rpoly": 0.9, + "qmaxinst": 29.0, + "qmin": 0.01, + "qmax": 27.0, + "rmin": 0.0, + "rmax": 60.0, + "rstep": 0.01, + "temperature": 300.0, + "bank": 0, + "nbanks": 1, + }, + ), + # C2: A neutron PDF written by PDFgetN. + # Expected: The header yields the neutron scattering type, + # the Q range and the rest of the xPDFsuite config. + ( + "ni-q27r100-neutron.gr", + { + "wavelength": 1.333, + "dataformat": "QA", + "inputfile": "npdf_03315.chi", + "backgroundfile": "npdf_03001.chi", + "stype": "N", + "bgscale": 1.0, + "composition": "Ni", + "outputtype": "gr", + "qmaxinst": 27.0, + "qmin": 0.87, + "qmax": 27.0, + "temperature": 300.0, + "rmax": 100.0, + "rmin": 0.0, + "rstep": 0.01, + "rpoly": 0.9, + "inputdir": "/data/npdf/chi", + "savedir": "/data/npdf/gr", + "bank": 0, + "nbanks": 1, + }, + ), + ], +) +def test_pdfparser_metadata(datafile, input_filename, expected_metadata): + """PDF specific metadata survives the load_data based parse_file.""" + parser = PDFParser() + parser.parse_file(datafile(input_filename)) + actual_metadata = parser.get_metadata() + # add the filename key to the expected metadata for comparison + expected_metadata["filename"] = str(datafile(input_filename)) + assert actual_metadata == expected_metadata + + +def test_pdfparser_deprecated_parseFile(datafile): + """The deprecated parseFile warns and delegates to parse_file.""" + input_filename = datafile("si-q27r60-xray.gr") + expected_parser = PDFParser() + expected_parser.parse_file(input_filename) + actual_parser = PDFParser() + with pytest.warns(DeprecationWarning): + actual_parser.parseFile(input_filename) + + actual_metadata = actual_parser.get_metadata() + expected_metadata = expected_parser.get_metadata() + assert actual_metadata == expected_metadata + + actual_x, actual_y, actual_dx, actual_dy = actual_parser.get_data() + expected_x, expected_y, expected_dx, expected_dy = ( + expected_parser.get_data() + ) + assert actual_x.tolist() == expected_x.tolist() + assert actual_y.tolist() == expected_y.tolist() + assert actual_dx == expected_dx + assert actual_dy.tolist() == expected_dy.tolist() diff --git a/tests/test_recipeorganizer.py b/tests/test_recipeorganizer.py index ac00dad5..1b953c3b 100644 --- a/tests/test_recipeorganizer.py +++ b/tests/test_recipeorganizer.py @@ -695,5 +695,116 @@ def test_register_function_introspects_through_a_decorator(input_function): assert actual_argnames == expected_argnames +def _make_iterpars_tree(): + """Build a small hierarchy for iterPars tests.""" + root = RecipeContainer("root") + root._containers = {} + root._manage(root._containers) + + root_biso = Parameter("Biso", 10) + root._add_object(root_biso, root._parameters) + + ni0 = RecipeContainer("Ni0") + ni0_biso = Parameter("Biso", 20) + ni0_uiso = Parameter("Uiso", 30) + ni0._add_object(ni0_biso, ni0._parameters) + ni0._add_object(ni0_uiso, ni0._parameters) + + ni1 = RecipeContainer("Ni1") + ni1_biso = Parameter("Biso", 40) + ni1._add_object(ni1_biso, ni1._parameters) + + o0 = RecipeContainer("O0") + o0_biso = Parameter("Biso", 50) + o0._add_object(o0_biso, o0._parameters) + + root._add_object(ni0, root._containers) + root._add_object(ni1, root._containers) + root._add_object(o0, root._containers) + + return { + "root": root, + "root_biso": root_biso, + "ni0": ni0, + "ni0_biso": ni0_biso, + "ni0_uiso": ni0_uiso, + "ni1": ni1, + "ni1_biso": ni1_biso, + "o0": o0, + "o0_biso": o0_biso, + } + + +@pytest.mark.parametrize( + ("pattern", "kwargs", "expected_values"), + [ + # C1: Match leaf parameter names without fullnames. + # Expected: all Biso parameters in the hierarchy are returned. + (r"^Biso$", {}, [10, 20, 40, 50]), + # C2: Match hierarchical names without fullnames. + # Expected: no leaf names match the hierarchical pattern. + (r"^Ni\d+\.Biso$", {}, []), + # C3: Match hierarchical names with fullnames enabled. + # Expected: matching Ni Biso parameters are returned. + (r"^Ni\d+\.Biso$", {"fullnames": True}, [20, 40]), + # C4: Match one hierarchical Uiso name. + # Expected: only Ni0.Uiso is returned. + (r"^Ni0\.Uiso$", {"fullnames": True}, [30]), + # C5: Match one hierarchical Biso name outside Ni containers. + # Expected: only O0.Biso is returned. + (r"^O0\.Biso$", {"fullnames": True}, [50]), + # C6: Disable recursion while matching child fullnames. + # Expected: no child parameters are returned. + (r"^Ni\d+\.Biso$", {"fullnames": True, "recurse": False}, []), + # C7: Disable recursion while matching root fullname. + # Expected: only the root-level Biso parameter is returned. + (r"^Biso$", {"fullnames": True, "recurse": False}, [10]), + ], +) +def test_iterpars_fullname_matching(pattern, kwargs, expected_values): + """Verify leaf-name and fullname matching in + iterate_over_parameters.""" + objs = _make_iterpars_tree() + root = objs["root"] + + actual_values = [ + parameter.value + for parameter in root.iterate_over_parameters(pattern, **kwargs) + ] + + assert actual_values == expected_values + + +@pytest.mark.parametrize( + ("pattern", "expected_name"), + [ + # C1: Match Biso relative to the called Ni0 container. + # Expected: Ni0.Biso is returned without the Ni0 prefix. + (r"^Biso$", ["Biso"]), + # C2: Match Uiso relative to the called Ni0 container. + # Expected: Ni0.Uiso is returned without the Ni0 prefix. + (r"^Uiso$", ["Uiso"]), + # C3: Match with the parent container prefix from inside Ni0. + # Expected: no parameter is returned because fullnames are relative + # to the container on which iterate_over_parameters is called. + (r"^Ni0\.Biso$", []), + ], +) +def test_iterpars_fullnames_are_relative_to_called_container( + pattern, + expected_name, +): + """Verify fullname matching is relative to the called container.""" + objs = _make_iterpars_tree() + ni0 = objs["ni0"] + + actual_name = [ + parameter.name + for parameter in ni0.iterate_over_parameters(pattern, fullnames=True) + ] + + assert actual_name == expected_name + + if __name__ == "__main__": unittest.main() From e83b05b2591b37a0f784f3874ea8fa47fb1f1e62 Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Fri, 14 Aug 2026 11:50:45 -0600 Subject: [PATCH 2/7] reorganize deprecated tests, no changes introduced to testing behavior --- tests/test_constraint.py | 13 ++- tests/test_fitrecipe.py | 213 ++++++++++++++++++++----------------- tests/test_parameterset.py | 32 ++++-- tests/test_pdfparser.py | 5 + tests/test_profile.py | 160 +++++++++++++++------------- 5 files changed, 238 insertions(+), 185 deletions(-) diff --git a/tests/test_constraint.py b/tests/test_constraint.py index f82c4384..1a87cdc3 100644 --- a/tests/test_constraint.py +++ b/tests/test_constraint.py @@ -60,10 +60,17 @@ def test_constrain_parameter(self): return -class TestConstraint_deprecated(unittest.TestCase): +# ---------------------------------------------------------------------------- +# Constraint.constrain and Parameter.setConst are deprecated in favor of +# Constraint.add_constraint and Parameter.set_constant. The old names must +# still work and forward to the new implementation. - def testConstraint(self): - """Test the Constraint class.""" + +class TestConstraintDeprecated(unittest.TestCase): + + def test_constrain_deprecated(self): + """Test the deprecated Constraint.constrain and + Parameter.setConst methods.""" p1 = Parameter("p1", 1) p2 = Parameter("p2", 2) diff --git a/tests/test_fitrecipe.py b/tests/test_fitrecipe.py index 3b51b857..022c27ca 100644 --- a/tests/test_fitrecipe.py +++ b/tests/test_fitrecipe.py @@ -340,62 +340,6 @@ def test_convert_bounds_to_restraints(): assert r.scaled is True -def testPrintFitHook(capturestdout): - "check output from default PrintFitHook." - recipe = FitRecipe("recipe") - recipe.fithooks[0].verbose = 0 - - # Set up the Profile - profile = Profile() - x = linspace(0, pi, 10) - y = sin(x) - profile.set_observed_profile(x, y) - - # Set up the FitContribution - fitcontribution = FitContribution("cont") - fitcontribution.set_profile(profile) - fitcontribution.set_equation("A*sin(k*x + c)") - fitcontribution.A.set_value(1) - fitcontribution.k.set_value(1) - fitcontribution.c.set_value(0) - - recipe.addContribution(fitcontribution) - - recipe.add_variable(fitcontribution.c) - recipe.add_soft_bounds("c", lower_bound=5) - (pfh,) = recipe.getFitHooks() - out = capturestdout(recipe.scalar_residual) - assert "" == out - pfh.verbose = 1 - out = capturestdout(recipe.scalar_residual) - assert out.strip().isdigit() - assert "\nRestraints:" not in out - pfh.verbose = 2 - out = capturestdout(recipe.scalar_residual) - assert "\nResidual:" in out - assert "\nRestraints:" in out - assert "\nVariables" not in out - pfh.verbose = 3 - out = capturestdout(recipe.scalarResidual) - assert "\nVariables" in out - assert "c = " in out - return - - -def test_add_and_remove_ParameterSet(): - # add a parset - recipe = FitRecipe("recipe") - parameter_to_add = Parameter("added_param", 1) - recipe.addParameterSet(parameter_to_add) - # check that the parameter is added - assert recipe.added_param == parameter_to_add - assert recipe.added_param.value == 1 - # remove the added parameter - recipe.removeParameterSet(parameter_to_add) - # check that the parameter is removed - assert not hasattr(recipe, "added_param") - - def test_add_and_remove_parameter_set(): recipe = FitRecipe("recipe") parameter_to_add = Parameter("added_param", 1) @@ -411,12 +355,12 @@ def test_add_and_remove_parameter_set(): def test_add_contribution(capturestdout): - """Duplicated test of PrintFitHooks except addContribution method - has changed to the new add_contribution method. This is because - addContribution is deprecated. + """Check output from default PrintFitHook using add_contribution. - Remove this test after addContribution is removed and update - testPrintFitHook to use add_contribution instead of addContribution. + Covers the same behavior as test_print_fit_hook_deprecated (see the + deprecated-API section at the bottom of this file), which uses the + deprecated addContribution instead. Once addContribution is removed + in 4.0.0, delete that test and keep this one. """ recipe = FitRecipe("recipe") recipe.fithooks[0].verbose = 0 @@ -668,27 +612,6 @@ def build_recipe_from_datafile(datafile): return recipe -def build_recipe_from_datafile_deprecated(datafile): - """Duplicate of build_recipe_from_datafile to use deprecated - loadParsedData method. - - Remove in version 4.0.0. - """ - profile = Profile() - parser = PDFParser() - parser.parseFile(str(datafile)) - profile.loadParsedData(parser) - - contribution = FitContribution("c") - contribution.set_profile(profile) - contribution.set_equation("m*x + b") - recipe = FitRecipe() - recipe.add_contribution(contribution) - recipe.add_variable(contribution.m, 1) - recipe.add_variable(contribution.b, 0) - return recipe - - def test_plot_recipe_bad_display(build_recipes_one_contribution): recipe, _ = build_recipes_one_contribution # Case: All plots are disabled @@ -1043,23 +966,6 @@ def test_plot_recipe_labels_from_gr_file_overwrite(temp_data_files): assert actual_ylabel == expected_ylabel -def test_plot_recipe_labels_from_gr_file_overwrite_deprecated(temp_data_files): - "Remove this test with version 4.0.0." - gr_file = temp_data_files / "gr_file.gr" - recipe = build_recipe_from_datafile_deprecated(gr_file) - optimize_recipe(recipe) - plt.close("all") - fig, ax = recipe.plot_recipe( - return_fig=True, show=False, xlabel="My X", ylabel="My Y" - ) - actual_xlabel = ax.get_xlabel() - actual_ylabel = ax.get_ylabel() - expected_xlabel = "My X" - expected_ylabel = "My Y" - assert actual_xlabel == expected_xlabel - assert actual_ylabel == expected_ylabel - - def test_plot_recipe_reset_all_defaults(build_recipes_one_contribution): expected_defaults = { "show_observed": True, @@ -1229,5 +1135,114 @@ def test_residual_is_weighted_by_uncertainty( assert actual_residual == pytest.approx(expected_residual) +# ---------------------------------------------------------------------------- +# addContribution, addParameterSet/removeParameterSet, and +# PDFParser.parseFile/Profile.loadParsedData are deprecated in favor of +# add_contribution, add_parameter_set/remove_parameter_set, and +# PDFParser.parse_file/Profile.load_parsed_data. The old names must still +# work and forward to the new implementation. + + +def test_print_fit_hook_deprecated(capturestdout): + """Check output from default PrintFitHook using the deprecated + addContribution, getFitHooks, and scalarResidual methods. + + Remove this test after addContribution is removed in 4.0.0; see + test_add_contribution for the permanent replacement. + """ + recipe = FitRecipe("recipe") + recipe.fithooks[0].verbose = 0 + + # Set up the Profile + profile = Profile() + x = linspace(0, pi, 10) + y = sin(x) + profile.set_observed_profile(x, y) + + # Set up the FitContribution + fitcontribution = FitContribution("cont") + fitcontribution.set_profile(profile) + fitcontribution.set_equation("A*sin(k*x + c)") + fitcontribution.A.set_value(1) + fitcontribution.k.set_value(1) + fitcontribution.c.set_value(0) + + recipe.addContribution(fitcontribution) + + recipe.add_variable(fitcontribution.c) + recipe.add_soft_bounds("c", lower_bound=5) + (pfh,) = recipe.getFitHooks() + out = capturestdout(recipe.scalar_residual) + assert "" == out + pfh.verbose = 1 + out = capturestdout(recipe.scalar_residual) + assert out.strip().isdigit() + assert "\nRestraints:" not in out + pfh.verbose = 2 + out = capturestdout(recipe.scalar_residual) + assert "\nResidual:" in out + assert "\nRestraints:" in out + assert "\nVariables" not in out + pfh.verbose = 3 + out = capturestdout(recipe.scalarResidual) + assert "\nVariables" in out + assert "c = " in out + return + + +def test_add_and_remove_parameter_set_deprecated(): + """Test the deprecated addParameterSet and removeParameterSet + methods.""" + recipe = FitRecipe("recipe") + parameter_to_add = Parameter("added_param", 1) + # add a parset + recipe.addParameterSet(parameter_to_add) + # check that the parameter is added + assert recipe.added_param == parameter_to_add + assert recipe.added_param.value == 1 + # remove the added parameter + recipe.removeParameterSet(parameter_to_add) + # check that the parameter is removed + assert not hasattr(recipe, "added_param") + + +def build_recipe_from_datafile_deprecated(datafile): + """Duplicate of build_recipe_from_datafile to use the deprecated + PDFParser.parseFile and Profile.loadParsedData methods. + + Remove in version 4.0.0. + """ + profile = Profile() + parser = PDFParser() + parser.parseFile(str(datafile)) + profile.loadParsedData(parser) + + contribution = FitContribution("c") + contribution.set_profile(profile) + contribution.set_equation("m*x + b") + recipe = FitRecipe() + recipe.add_contribution(contribution) + recipe.add_variable(contribution.m, 1) + recipe.add_variable(contribution.b, 0) + return recipe + + +def test_plot_recipe_labels_from_gr_file_overwrite_deprecated(temp_data_files): + "Remove this test with version 4.0.0." + gr_file = temp_data_files / "gr_file.gr" + recipe = build_recipe_from_datafile_deprecated(gr_file) + optimize_recipe(recipe) + plt.close("all") + fig, ax = recipe.plot_recipe( + return_fig=True, show=False, xlabel="My X", ylabel="My Y" + ) + actual_xlabel = ax.get_xlabel() + actual_ylabel = ax.get_ylabel() + expected_xlabel = "My X" + expected_ylabel = "My Y" + assert actual_xlabel == expected_xlabel + assert actual_ylabel == expected_ylabel + + if __name__ == "__main__": unittest.main() diff --git a/tests/test_parameterset.py b/tests/test_parameterset.py index 1a954c8d..1838c2f4 100644 --- a/tests/test_parameterset.py +++ b/tests/test_parameterset.py @@ -26,16 +26,12 @@ def setUp(self): self.parset = ParameterSet("test") return - def testAddParameterSet(self): - """Test the deprecated addParameterSet method. - - Remove this test after the addParameterSet is removed in version - 4.0.0. - """ + def test_add_parameter_set(self): + """Test the add_parameter_set method.""" parset2 = ParameterSet("parset2") p1 = Parameter("parset2", 1) - self.parset.addParameterSet(parset2) + self.parset.add_parameter_set(parset2) self.assertTrue(self.parset.parset2 is parset2) self.assertRaises(ValueError, self.parset.add_parameter_set, p1) @@ -47,12 +43,28 @@ def testAddParameterSet(self): return - def test_add_parameter_set(self): - """Test the add_parameter_set method.""" + +# ---------------------------------------------------------------------------- +# addParameterSet is deprecated in favor of add_parameter_set. The old name +# must still work and forward to the new implementation. + + +class TestParameterSetDeprecated(unittest.TestCase): + + def setUp(self): + self.parset = ParameterSet("test") + return + + def test_add_parameter_set_deprecated(self): + """Test the deprecated addParameterSet method. + + Remove this test after the addParameterSet is removed in version + 4.0.0. + """ parset2 = ParameterSet("parset2") p1 = Parameter("parset2", 1) - self.parset.add_parameter_set(parset2) + self.parset.addParameterSet(parset2) self.assertTrue(self.parset.parset2 is parset2) self.assertRaises(ValueError, self.parset.add_parameter_set, p1) diff --git a/tests/test_pdfparser.py b/tests/test_pdfparser.py index 65bc1114..02b7bea5 100644 --- a/tests/test_pdfparser.py +++ b/tests/test_pdfparser.py @@ -178,6 +178,11 @@ def test_pdfparser_metadata(datafile, input_filename, expected_metadata): assert actual_metadata == expected_metadata +# ---------------------------------------------------------------------------- +# parseFile is deprecated in favor of parse_file. The old name must still +# work, emit a DeprecationWarning, and forward to the new implementation. + + def test_pdfparser_deprecated_parseFile(datafile): """The deprecated parseFile warns and delegates to parse_file.""" input_filename = datafile("si-q27r60-xray.gr") diff --git a/tests/test_profile.py b/tests/test_profile.py index 54b47d20..59ab81d1 100644 --- a/tests/test_profile.py +++ b/tests/test_profile.py @@ -77,42 +77,6 @@ def test_set_observed_profile(self): return - def testSetObservedProfile(self): - """Test the deprecated setObservedProfile method. - - Remove this test when setObservedProfile is removed in 4.0.0. - """ - # Make a profile with defined dy - - x = arange(0, 10, 0.1) - y = x - dy = x - - prof = self.profile - prof.set_observed_profile(x, y, dy) - - self.assertTrue(array_equal(x, prof.xobs)) - self.assertTrue(array_equal(y, prof.yobs)) - self.assertTrue(array_equal(dy, prof.dyobs)) - - # Make a profile with undefined dy - x = arange(0, 10, 0.1) - y = x - dy = None - - self.profile.setObservedProfile(x, y, dy) - - self.assertTrue(array_equal(x, prof.xobs)) - self.assertTrue(array_equal(y, prof.yobs)) - self.assertTrue(prof.dyobs is None) - - # Get the ranged profile to make sure its the same - self.assertTrue(array_equal(x, prof.x)) - self.assertTrue(array_equal(y, prof.y)) - self.assertTrue(array_equal(ones_like(prof.xobs), prof.dy)) - - return - def test_set_calculation_range(self): """Test the set_calculation_range method.""" x = arange(2, 9.6, 0.5) @@ -197,26 +161,6 @@ def test_set_calculation_range(self): self.assertTrue(array_equal(prof.x, arange(4.5, 6.1, 0.5))) return - def testSetCalculationRange(self): - """Test the deprecated setCalculationRange method. - - Remove this test when setCalculationRange is removed in 4.0.0. - """ - x = arange(2, 9.6, 0.5) - y = array(x) - dy = array(x) - prof = self.profile - prof.set_observed_profile(x, y, dy) - # Test normal execution w/o arguments - self.assertTrue(array_equal(x, prof.x)) - self.assertTrue(array_equal(y, prof.y)) - self.assertTrue(array_equal(dy, prof.dy)) - prof.setCalculationRange() - self.assertTrue(array_equal(x, prof.x)) - self.assertTrue(array_equal(y, prof.y)) - self.assertTrue(array_equal(dy, prof.dy)) - return - def test_set_calculation_points(self): """Test the set_calculation_points method.""" prof = self.profile @@ -236,7 +180,93 @@ def test_set_calculation_points(self): return - def testSetCalculationPoints(self): + def test_savetxt(self): + "Check the savetxt method." + prof = self.profile + self.assertRaises(SrFitError, prof.savetxt, "foo") + xobs = arange(-2, 3.01, 0.25) + yobs = xobs**2 + prof.set_observed_profile(xobs, yobs) + prof.ycalc = yobs.copy() + fp = io.BytesIO() + prof.savetxt(fp) + txt = fp.getvalue().decode() + self.assertTrue(re.match(r"^# x +ycalc +y +dy\b", txt)) + nlines = len(txt.strip().split("\n")) + self.assertEqual(22, nlines) + return + + +# ---------------------------------------------------------------------------- +# setObservedProfile, setCalculationRange, and setCalculationPoints are +# deprecated in favor of set_observed_profile, set_calculation_range, and +# set_calculation_points. The old names must still work and forward to the +# new implementation. + + +class TestProfileDeprecated(unittest.TestCase): + + def setUp(self): + self.profile = Profile() + return + + def test_set_observed_profile_deprecated(self): + """Test the deprecated setObservedProfile method. + + Remove this test when setObservedProfile is removed in 4.0.0. + """ + # Make a profile with defined dy + + x = arange(0, 10, 0.1) + y = x + dy = x + + prof = self.profile + prof.set_observed_profile(x, y, dy) + + self.assertTrue(array_equal(x, prof.xobs)) + self.assertTrue(array_equal(y, prof.yobs)) + self.assertTrue(array_equal(dy, prof.dyobs)) + + # Make a profile with undefined dy + x = arange(0, 10, 0.1) + y = x + dy = None + + self.profile.setObservedProfile(x, y, dy) + + self.assertTrue(array_equal(x, prof.xobs)) + self.assertTrue(array_equal(y, prof.yobs)) + self.assertTrue(prof.dyobs is None) + + # Get the ranged profile to make sure its the same + self.assertTrue(array_equal(x, prof.x)) + self.assertTrue(array_equal(y, prof.y)) + self.assertTrue(array_equal(ones_like(prof.xobs), prof.dy)) + + return + + def test_set_calculation_range_deprecated(self): + """Test the deprecated setCalculationRange method. + + Remove this test when setCalculationRange is removed in 4.0.0. + """ + x = arange(2, 9.6, 0.5) + y = array(x) + dy = array(x) + prof = self.profile + prof.set_observed_profile(x, y, dy) + # Test normal execution w/o arguments + self.assertTrue(array_equal(x, prof.x)) + self.assertTrue(array_equal(y, prof.y)) + self.assertTrue(array_equal(dy, prof.dy)) + prof.setCalculationRange() + self.assertTrue(array_equal(x, prof.x)) + self.assertTrue(array_equal(y, prof.y)) + self.assertTrue(array_equal(dy, prof.dy)) + return + + def test_set_calculation_points_deprecated(self): """Test the deprecated setCalculationPoints method. Remove this test when setCalculationPoints is removed in 4.0.0. @@ -258,22 +288,6 @@ def testSetCalculationPoints(self): return - def test_savetxt(self): - "Check the savetxt method." - prof = self.profile - self.assertRaises(SrFitError, prof.savetxt, "foo") - xobs = arange(-2, 3.01, 0.25) - yobs = xobs**2 - prof.set_observed_profile(xobs, yobs) - prof.ycalc = yobs.copy() - fp = io.BytesIO() - prof.savetxt(fp) - txt = fp.getvalue().decode() - self.assertTrue(re.match(r"^# x +ycalc +y +dy\b", txt)) - nlines = len(txt.strip().split("\n")) - self.assertEqual(22, nlines) - return - def testLoadtxt(datafile): """Test the loadtxt method.""" From 30c5964ed2a0ab2eb7611e1e328b219a7687045e Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Fri, 14 Aug 2026 12:02:21 -0600 Subject: [PATCH 3/7] test: add test for parsing pdf metadata from NOMAD file --- tests/test_pdfparser.py | 27 +++++++++++++++++++++++++++ tests/testdata/nom-mno-neutron.gr | 15 +++++++++++++++ 2 files changed, 42 insertions(+) create mode 100644 tests/testdata/nom-mno-neutron.gr diff --git a/tests/test_pdfparser.py b/tests/test_pdfparser.py index 02b7bea5..35a58570 100644 --- a/tests/test_pdfparser.py +++ b/tests/test_pdfparser.py @@ -178,6 +178,33 @@ def test_pdfparser_metadata(datafile, input_filename, expected_metadata): assert actual_metadata == expected_metadata +# ---------------------------------------------------------------------------- +# NOMAD (NOM) files at SNS prepend free-text comments instead of plain +# name = value pairs, e.g.: +# # Comment: neutron, Qmax=31.414, Qdamp=0.017659, Qbroad= 0.0191822 +# PDFParser must still recognize the scattering type and pull qmax, +# qdamp, and qbroad out of that comment, or refinements built from +# these files silently lose their resolution parameters. + + +def test_pdfparser_nomad_comment_metadata(datafile): + """PDFParser recovers stype, qmax, qdamp, and qbroad from the free- + text NOMAD instrument comment header.""" + parser = PDFParser() + parser.parse_file(datafile("nom-mno-neutron.gr")) + actual_metadata = parser.get_metadata() + expected_metadata = { + "stype": "N", + "qmax": 31.414, + "qdamp": 0.017659, + "qbroad": 0.0191822, + } + assert actual_metadata["stype"] == expected_metadata["stype"] + assert actual_metadata["qmax"] == expected_metadata["qmax"] + assert actual_metadata["qdamp"] == expected_metadata["qdamp"] + assert actual_metadata["qbroad"] == expected_metadata["qbroad"] + + # ---------------------------------------------------------------------------- # parseFile is deprecated in favor of parse_file. The old name must still # work, emit a DeprecationWarning, and forward to the new implementation. diff --git a/tests/testdata/nom-mno-neutron.gr b/tests/testdata/nom-mno-neutron.gr new file mode 100644 index 00000000..7a0b64cb --- /dev/null +++ b/tests/testdata/nom-mno-neutron.gr @@ -0,0 +1,15 @@ +# 5000 +# file: PDF/NOM_9999_MnO_5K_ftfrgr.gr +# created: Thu Sep 29 18:48:08 2016 +# Comment: neutron, Qmax=31.414, Qdamp=0.017659, Qbroad= 0.0191822 +# + 0.01 0.000 + 0.02 0.010 + 0.03 0.020 + 0.04 0.030 + 0.05 0.040 + 0.06 0.050 + 0.07 0.060 + 0.08 0.070 + 0.09 0.080 + 0.10 0.090 From fb03d3f0d8e0d7709b9e8bc3d9c83d45b3e9745b Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Fri, 14 Aug 2026 12:16:50 -0600 Subject: [PATCH 4/7] move test into parametrized testing function --- tests/test_pdfparser.py | 59 +++++++++++++++++------------------------ 1 file changed, 25 insertions(+), 34 deletions(-) diff --git a/tests/test_pdfparser.py b/tests/test_pdfparser.py index 35a58570..cc064f38 100644 --- a/tests/test_pdfparser.py +++ b/tests/test_pdfparser.py @@ -105,12 +105,13 @@ def test_pdfparser_data( assert actual_dy == approx_or_none(expected_dy) -# PDFParser inherits ProfileParser's hooks unchanged: PDFgetX and -# PDFgetN headers are already plain name = value pairs, including -# stype = X or stype = N for the scattering type. The metadata below -# reaches PDFGenerator, which uses stype, qmin and qmax to set the -# scattering type and the Q range, so losing a key silently changes a -# refinement. +# PDFParser inherits ProfileParser's hooks for plain name = value +# headers, including stype = X or stype = N for the scattering type, +# and falls back to scanning free-text instrument comments (e.g. +# NOMAD at SNS) for stype, qmax, qdamp, and qbroad when those are not +# already name = value pairs. The metadata below reaches PDFGenerator, +# which uses stype, qmin and qmax to set the scattering type and the Q +# range, so losing a key silently changes a refinement. @pytest.mark.parametrize( "input_filename, expected_metadata", [ @@ -166,10 +167,27 @@ def test_pdfparser_data( "nbanks": 1, }, ), + # C3: A neutron PDF written for the NOMAD instrument at SNS, + # whose header has no name = value pairs at all, only a + # free-text instrument comment. + # Expected: The comment yields the neutron scattering type + # and the qmax, qdamp, and qbroad resolution parameters. + ( + "nom-mno-neutron.gr", + { + "stype": "N", + "qmax": 31.414, + "qdamp": 0.017659, + "qbroad": 0.0191822, + "bank": 0, + "nbanks": 1, + }, + ), ], ) def test_pdfparser_metadata(datafile, input_filename, expected_metadata): - """PDF specific metadata survives the load_data based parse_file.""" + """PDF specific metadata survives the load_data based parse_file, + including free-text instrument comment headers.""" parser = PDFParser() parser.parse_file(datafile(input_filename)) actual_metadata = parser.get_metadata() @@ -178,33 +196,6 @@ def test_pdfparser_metadata(datafile, input_filename, expected_metadata): assert actual_metadata == expected_metadata -# ---------------------------------------------------------------------------- -# NOMAD (NOM) files at SNS prepend free-text comments instead of plain -# name = value pairs, e.g.: -# # Comment: neutron, Qmax=31.414, Qdamp=0.017659, Qbroad= 0.0191822 -# PDFParser must still recognize the scattering type and pull qmax, -# qdamp, and qbroad out of that comment, or refinements built from -# these files silently lose their resolution parameters. - - -def test_pdfparser_nomad_comment_metadata(datafile): - """PDFParser recovers stype, qmax, qdamp, and qbroad from the free- - text NOMAD instrument comment header.""" - parser = PDFParser() - parser.parse_file(datafile("nom-mno-neutron.gr")) - actual_metadata = parser.get_metadata() - expected_metadata = { - "stype": "N", - "qmax": 31.414, - "qdamp": 0.017659, - "qbroad": 0.0191822, - } - assert actual_metadata["stype"] == expected_metadata["stype"] - assert actual_metadata["qmax"] == expected_metadata["qmax"] - assert actual_metadata["qdamp"] == expected_metadata["qdamp"] - assert actual_metadata["qbroad"] == expected_metadata["qbroad"] - - # ---------------------------------------------------------------------------- # parseFile is deprecated in favor of parse_file. The old name must still # work, emit a DeprecationWarning, and forward to the new implementation. From 4689b4e67e92ab3e01804ad355db657ce2f01dda Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Fri, 14 Aug 2026 12:17:11 -0600 Subject: [PATCH 5/7] feat: add parsing ability for NOMAD file header --- src/diffpy/srfit/pdf/pdfparser.py | 76 ++++++++++++++++++++++++++++++- 1 file changed, 75 insertions(+), 1 deletion(-) diff --git a/src/diffpy/srfit/pdf/pdfparser.py b/src/diffpy/srfit/pdf/pdfparser.py index 26f5c584..257f392d 100644 --- a/src/diffpy/srfit/pdf/pdfparser.py +++ b/src/diffpy/srfit/pdf/pdfparser.py @@ -22,8 +22,13 @@ __all__ = ["PDFParser"] +import re +from pathlib import Path + from diffpy.srfit.fitbase.profileparser import ProfileParser +_FLOAT_RX = r"[-+]?(?:\d+(?:\.\d*)?|\.\d+)(?:[eE][-+]?\d+)?" + class PDFParser(ProfileParser): """Parser for PDF diffraction pattern data. @@ -31,7 +36,11 @@ class PDFParser(ProfileParser): PDFgetX and PDFgetN write their header as plain ``name = value`` pairs, including ``stype = X`` or ``stype = N`` for the scattering type, so this class parses files identically to ``ProfileParser`` - and only sets ``_format`` to identify PDF data. + for those. Some facilities (e.g. NOMAD at SNS) instead prepend a + free-text instrument comment, so this class also falls back to + scanning that comment for the scattering type, ``qmin``, ``qmax``, + ``qdamp``, and ``qbroad`` when they are not already present as + ``name = value`` pairs. Attributes ---------- @@ -85,6 +94,10 @@ class PDFParser(ProfileParser): The minimum scattering vector (float). qmax The maximum scattering vector (float). + qdamp + The Q-resolution damping factor (float). + qbroad + The Q-resolution broadening factor (float). These, along with any other ``name = value`` pairs in the header, may appear in the metadata dictionary. @@ -92,5 +105,66 @@ class PDFParser(ProfileParser): _format = "PDF" + def _parse_metadata(self, filename): + """Return the metadata read from a PDFgetX or PDFgetN header. + + This calls ``ProfileParser``'s ``name = value`` based parsing + first, then falls back to scanning the free-text instrument + comments some facilities prepend to their files for the + scattering type and Q-resolution parameters that + such comments are not already covered by a ``name = value`` + pair. + + Parameters + ---------- + filename : str or Path + The name of the file to parse. + + Returns + ------- + dict + The metadata read from the file header. + """ + metadata = super()._parse_metadata(filename) + self._parse_comment_metadata(Path(filename).read_text(), metadata) + return metadata + + @staticmethod + def _parse_comment_metadata(header_text, metadata): + """Fill in stype, qmin, qmax, qdamp, and qbroad from free-text + instrument comments, without overwriting values already found + by the ``name = value`` based parsing. + + Parameters + ---------- + header_text : str + The full text of the file being parsed. + meta : dict + The metadata dictionary to update in place. + + Returns + ------- + dict + The updated metadata dictionary. + """ + if "stype" not in metadata: + if re.search(r"(x-?ray|PDFgetX)", header_text, re.I): + metadata["stype"] = "X" + elif re.search(r"(neutron|PDFgetN)", header_text, re.I): + metadata["stype"] = "N" + regexes = { + "qmin": r"\bqmin *= *(%s)\b" % _FLOAT_RX, + "qmax": r"\bqmax *= *(%s)\b" % _FLOAT_RX, + "qdamp": r"\b(?:qdamp|qsig) *= *(%s)\b" % _FLOAT_RX, + "qbroad": r"\b(?:qbroad|qalp) *= *(%s)\b" % _FLOAT_RX, + } + for key, pattern in regexes.items(): + if key in metadata: + continue + res = re.search(pattern, header_text, re.I) + if res: + metadata[key] = float(res.group(1)) + return metadata + # End of PDFParser From a64feb1443b4ba61fa33833615c9f8f37b0c2768 Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Fri, 14 Aug 2026 12:20:40 -0600 Subject: [PATCH 6/7] fix docstring --- src/diffpy/srfit/pdf/pdfparser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/diffpy/srfit/pdf/pdfparser.py b/src/diffpy/srfit/pdf/pdfparser.py index 257f392d..e9655a3b 100644 --- a/src/diffpy/srfit/pdf/pdfparser.py +++ b/src/diffpy/srfit/pdf/pdfparser.py @@ -36,7 +36,7 @@ class PDFParser(ProfileParser): PDFgetX and PDFgetN write their header as plain ``name = value`` pairs, including ``stype = X`` or ``stype = N`` for the scattering type, so this class parses files identically to ``ProfileParser`` - for those. Some facilities (e.g. NOMAD at SNS) instead prepend a + for those. Some facilities instead prepend a free-text instrument comment, so this class also falls back to scanning that comment for the scattering type, ``qmin``, ``qmax``, ``qdamp``, and ``qbroad`` when they are not already present as From 4d7cbfd803893e4574937c930e3ed7fa26eb9eb9 Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Fri, 14 Aug 2026 12:25:22 -0600 Subject: [PATCH 7/7] news --- news/test-refactor.rst | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 news/test-refactor.rst diff --git a/news/test-refactor.rst b/news/test-refactor.rst new file mode 100644 index 00000000..d6b69c3e --- /dev/null +++ b/news/test-refactor.rst @@ -0,0 +1,23 @@ +**Added:** + +* Add regex parsing ability in ``PDFParser`` for file headers that do not have the structure output by xPDFsuite and PDFgetX. + +**Changed:** + +* + +**Deprecated:** + +* + +**Removed:** + +* + +**Fixed:** + +* + +**Security:** + +*