From 8607d55a4ef4de4550303350da73e36c6860a7b7 Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Mon, 14 Sep 2026 16:21:17 -0400 Subject: [PATCH] fix: restore PDFParser so diffpy.cmipdf is importable The package __init__ imported diffpy.cmipdf.pdfparser and listed PDFParser in __all__, but the module had been removed, so `import diffpy.cmipdf` raised ModuleNotFoundError and the whole test suite failed at collection. Port pdfparser.py from diffpy.srfit, which is the authoritative copy: its PDFParser was refactored onto ProfileParser's parse_file template with _parse_metadata/_parse_data hooks and gained free-text instrument comment parsing for NOMAD files. No deprecation shims are carried over, since diffpy.cmipdf has never been released. tests/test_parser.py exercised diffpy.srfit's ProfileParser and asserted the pre-refactor behaviour that dx is an array of zeros; it is replaced by tests/test_pdfparser.py, ported from diffpy.srfit, which tests this package's PDFParser and expects dx to be None. Metadata expectations are adapted to this repo's synthetic .gr fixtures rather than overwriting them. Adds nom-mno-neutron.gr for the NOMAD comment-header case and the as_list fixture the ported tests need. Co-Authored-By: Claude Opus 5 --- news/fix-pdfparser-import.rst | 23 ++++ src/diffpy/cmipdf/pdfparser.py | 171 ++++++++++++++++++++++++ tests/conftest.py | 9 ++ tests/test_parser.py | 139 -------------------- tests/test_pdfparser.py | 207 ++++++++++++++++++++++++++++++ tests/testdata/nom-mno-neutron.gr | 15 +++ 6 files changed, 425 insertions(+), 139 deletions(-) create mode 100644 news/fix-pdfparser-import.rst create mode 100644 src/diffpy/cmipdf/pdfparser.py delete mode 100644 tests/test_parser.py create mode 100644 tests/test_pdfparser.py create mode 100644 tests/testdata/nom-mno-neutron.gr diff --git a/news/fix-pdfparser-import.rst b/news/fix-pdfparser-import.rst new file mode 100644 index 0000000..7db7658 --- /dev/null +++ b/news/fix-pdfparser-import.rst @@ -0,0 +1,23 @@ +**Added:** + +* No news added: Restores a module the package __init__ already referenced; no user-facing change since diffpy.cmipdf has not been released. + +**Changed:** + +* + +**Deprecated:** + +* + +**Removed:** + +* + +**Fixed:** + +* + +**Security:** + +* diff --git a/src/diffpy/cmipdf/pdfparser.py b/src/diffpy/cmipdf/pdfparser.py new file mode 100644 index 0000000..e17c112 --- /dev/null +++ b/src/diffpy/cmipdf/pdfparser.py @@ -0,0 +1,171 @@ +#!/usr/bin/env python +############################################################################## +# +# (c) 2025 Simon Billinge. +# All rights reserved. +# +# File coded by: Caden Myers, Simon Billinge, and members of the Billinge +# group. +# +# See GitHub contributions for a more detailed list of contributors. +# https://github.com/diffpy/diffpy.cmipdf/graphs/contributors +# +# See LICENSE.rst for license information. +# +############################################################################## +"""This module contains parsers for PDF data. + +PDFParser is suitable for parsing data generated from PDFGetN and +PDFGetX. + +See the class documentation for more information. +""" + +__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. + + 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 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 + ---------- + _format + The name of the data format that this parses (string, default + ``""``). The format string is a unique identifier for the data + format handled by the parser. + _banks + The data from each bank. Each bank contains a + (x, y, dx, dy) tuple: + x + A numpy array containing the independent + variable read from the file. + y + A numpy array containing the profile + from the file. + dx + A numpy array containing the uncertainty in x + read from the file. This is None if the + uncertainty cannot be read. + dy + A numpy array containing the uncertainty read + from the file. This is None if the uncertainty + cannot be read. + _x + The independent variable from the chosen bank. + _y + The profile from the chosen bank. + _dx + The uncertainty in independent variable from the chosen bank. + _dy + The uncertainty in profile from the chosen bank. + _meta + A dictionary containing metadata read from the file. + + General Metadata + ----------------- + filename + The name of the file from which data was parsed. This key + will not exist if data was not read from file. + nbanks + The number of banks parsed. + bank + The chosen bank number. + + Metadata + -------- + stype + The scattering type ("X", "N"). + qmin + 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. + """ + + _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 diff --git a/tests/conftest.py b/tests/conftest.py index 937867d..9245c31 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -72,3 +72,12 @@ def _datafile(filename): return importlib.resources.files("tests.testdata").joinpath(filename) return _datafile + + +@pytest.fixture(scope="session") +def as_list(): + def _as_list(values): + """Unavailable uncertainties are None rather than an array.""" + return None if values is None else values.tolist() + + return _as_list diff --git a/tests/test_parser.py b/tests/test_parser.py deleted file mode 100644 index 3c7952c..0000000 --- a/tests/test_parser.py +++ /dev/null @@ -1,139 +0,0 @@ -#!/usr/bin/env python -############################################################################## -# -# (c) 2025 Simon Billinge. -# All rights reserved. -# -# File coded by: Caden Myers, Simon Billinge, and members of the Billinge -# group. -# -# See GitHub contributions for a more detailed list of contributors. -# https://github.com/diffpy/diffpy.cmipdf/graphs/contributors -# -# See LICENSE.rst for license information. -# -############################################################################## -"""Tests for pdf package.""" - - -import numpy as np -import pytest - -from diffpy.srfit.fitbase import ProfileParser - -# ---------------------------------------------------------------------------- - - -# The tests in this file are for ProfileParser which belongs to diffpy.srfit, -# but since it is used here, we will test it here on test data with modern -# diffpy format. -def testParser1(datafile): - filename = datafile("ni-q27r100-neutron.gr") - parser = ProfileParser() - parser.parse_file(filename) - - meta = parser._meta - - assert str(filename) == meta["filename"] - assert 1 == meta["nbanks"] - assert "N" == meta["stype"] - assert 27 == meta["qmax"] - assert 300 == meta.get("temperature") - assert meta.get("qdamp") is None - assert meta.get("qbroad") is None - assert meta.get("spdiameter") is None - assert meta.get("scale") is None - assert meta.get("doping") is None - - x, y, dx, dy = parser.get_data() - assert dx.tolist() == len(x) * [0] - assert dy.tolist() == len(x) * [0] - - testx = np.linspace(0.01, 100, 10000) - diff = testx - x - res = np.dot(diff, diff) - assert 0 == pytest.approx(res) - - testy = np.array( - [ - 1.144, - 2.258, - 3.312, - 4.279, - 5.135, - 5.862, - 6.445, - 6.875, - 7.150, - 7.272, - ] - ) - diff = testy - y[:10] - res = np.dot(diff, diff) - assert 0 == pytest.approx(res) - - return - - -def testParser2(datafile): - data = datafile("si-q27r60-xray.gr") - parser = ProfileParser() - parser.parse_file(data) - - meta = parser._meta - - assert str(data) == meta["filename"] - assert 1 == meta["nbanks"] - assert "X" == meta["stype"] - assert 27 == meta["qmax"] - assert 300 == meta.get("temperature") - assert meta.get("qdamp") is None - assert meta.get("qbroad") is None - assert meta.get("spdiameter") is None - assert meta.get("scale") is None - assert meta.get("doping") is None - - x, y, dx, dy = parser.get_data() - testx = np.linspace(0.01, 60, 5999, endpoint=False) - diff = testx - x - res = np.dot(diff, diff) - assert 0 == pytest.approx(res) - - testy = np.array( - [ - 0.1105784, - 0.2199684, - 0.3270088, - 0.4305913, - 0.5296853, - 0.6233606, - 0.7108060, - 0.7913456, - 0.8644501, - 0.9297440, - ] - ) - diff = testy - y[:10] - res = np.dot(diff, diff) - assert 0 == pytest.approx(res) - - testdy = np.array( - [ - 0.001802192, - 0.003521449, - 0.005079115, - 0.006404892, - 0.007440527, - 0.008142955, - 0.008486813, - 0.008466340, - 0.008096858, - 0.007416456, - ] - ) - diff = testdy - dy[:10] - res = np.dot(diff, diff) - assert 0 == pytest.approx(res) - - assert dx.tolist() == [0] * len(dx) - return diff --git a/tests/test_pdfparser.py b/tests/test_pdfparser.py new file mode 100644 index 0000000..0cccaf9 --- /dev/null +++ b/tests/test_pdfparser.py @@ -0,0 +1,207 @@ +#!/usr/bin/env python +############################################################################## +# +# (c) 2025 Simon Billinge. +# All rights reserved. +# +# File coded by: Caden Myers, Simon Billinge, and members of the Billinge +# group. +# +# See GitHub contributions for a more detailed list of contributors. +# https://github.com/diffpy/diffpy.cmipdf/graphs/contributors +# +# See LICENSE.rst for license information. +# +############################################################################## +"""Tests for the pdfparser module.""" + +import numpy +import pytest + +from diffpy.cmipdf.pdfparser 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 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", + [ + # C1: An x-ray PDF written by PDFgetX2, whose header is the + # synthetic xPDFsuite config used as a fixture in this repo. + # Expected: The header yields the x-ray scattering type, the Q + # range and the rest of the config. + ( + "si-q27r60-xray.gr", + { + "stype": "X", + "rmax": 60.0, + "wavelength": 0.1, + "dataformat": "QA", + "inputfile": "input.iq", + "backgroundfile": "backgroundfile.iq", + "backgroundfilefull": "/my/data/dir/backgroundfile.iq", + "mode": "neutron", + "bgscale": 1.0, + "composition": "TiSe2", + "outputtype": "gr", + "qmaxinst": 25.0, + "qmin": 0.1, + "qmax": 27.0, + "rmin": 0.0, + "rstep": 0.01, + "rpoly": 0.7, + "temperature": 300.0, + "inputdir": "/my/data/dir", + "savedir": "/my/save/dir", + "bank": 0, + "nbanks": 1, + }, + ), + # C2: A neutron PDF written by PDFgetN, whose header is the + # synthetic xPDFsuite config used as a fixture in this repo. + # Expected: The header yields the neutron scattering type, the Q + # range and the rest of the config. + ( + "ni-q27r100-neutron.gr", + { + "stype": "N", + "rmax": 100.0, + "wavelength": 0.1, + "dataformat": "QA", + "inputfile": "input.iq", + "backgroundfile": "backgroundfile.iq", + "backgroundfilefull": "/my/data/dir/backgroundfile.iq", + "mode": "neutron", + "bgscale": 1.0, + "composition": "TiSe2", + "outputtype": "gr", + "qmaxinst": 25.0, + "qmin": 0.1, + "qmax": 27.0, + "rmin": 0.0, + "rstep": 0.01, + "rpoly": 0.7, + "temperature": 300.0, + "inputdir": "/my/data/dir", + "savedir": "/my/save/dir", + "bank": 0, + "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, + including free-text instrument comment headers.""" + 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 diff --git a/tests/testdata/nom-mno-neutron.gr b/tests/testdata/nom-mno-neutron.gr new file mode 100644 index 0000000..7a0b64c --- /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