From 6237734aac6104f546fbd552ba29d5a3a8999d35 Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Thu, 30 Jul 2026 10:21:01 -0400 Subject: [PATCH 01/11] fix: change pushFitHooks to push_fit_hooks in one spot that was missed --- src/diffpy/srfit/fitbase/fitrecipe.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/diffpy/srfit/fitbase/fitrecipe.py b/src/diffpy/srfit/fitbase/fitrecipe.py index 101c88b8..93ab1b78 100644 --- a/src/diffpy/srfit/fitbase/fitrecipe.py +++ b/src/diffpy/srfit/fitbase/fitrecipe.py @@ -224,7 +224,7 @@ def __init__(self, name="fit"): """Initialization.""" RecipeOrganizer.__init__(self, name) self.fithooks = [] - self.pushFitHook(PrintFitHook()) + self.push_fit_hook(PrintFitHook()) self._restraintlist = [] self._oconstraints = [] self._ready = False From 761e2298075c3612e42ad3017d66018c8b1793b4 Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Mon, 3 Aug 2026 11:06:07 -0400 Subject: [PATCH 02/11] docs: fix stale claims in profileparser deprecation news entry --- news/profileparser_dep.rst | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/news/profileparser_dep.rst b/news/profileparser_dep.rst index 9f180049..20a00f59 100644 --- a/news/profileparser_dep.rst +++ b/news/profileparser_dep.rst @@ -1,11 +1,10 @@ **Added:** * Add ``parse_file`` method to ``ProfileParser`` to parse a file directly with ``load_data`` from ``diffpy.utils``. -* Add ``get_num_bank`` method to ``ProfileParser`` to replace ``getNumBank``. +* Add ``get_num_banks`` method to ``ProfileParser`` to replace ``getNumBanks``. * Add ``select_bank`` method to ``ProfileParser`` to replace ``selectBank``. -* Add ``get_format`` method to ``ProfileParser`` to replace ``getFormat``. * Add ``get_data`` method to ``ProfileParser`` to replace ``getData``. -* Add ``get_meta_data`` method to ``ProfileParser`` to replace ``getMetaData``. +* Add ``get_metadata`` method to ``ProfileParser`` to replace ``getMetaData``. **Changed:** @@ -13,8 +12,7 @@ **Deprecated:** -* Deprecate ``PDFParser``. Use ``ProfileParser`` instead. -* Deprecate ``getNumBank``, ``selectBank``, ``getFormat``, ``getData``, and ``getMetaData`` in ``ProfileParser``. +* Deprecate ``getNumBanks``, ``selectBank``, ``getData``, and ``getMetaData`` in ``ProfileParser``. **Removed:** From 532ac1de57e34822b01e0bd87d1af4acacbcab4b Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Mon, 3 Aug 2026 11:07:23 -0400 Subject: [PATCH 03/11] fix: merge ProfileParser.parseFile and parse_file into single dispatching method --- docs/source/extending.rst | 22 ++-- src/diffpy/srfit/fitbase/profileparser.py | 123 ++++++++++++++-------- src/diffpy/srfit/pdf/pdfparser.py | 20 +--- tests/test_pdf.py | 86 ++++++++++++++- 4 files changed, 177 insertions(+), 74 deletions(-) diff --git a/docs/source/extending.rst b/docs/source/extending.rst index 164ef187..388cf0ce 100644 --- a/docs/source/extending.rst +++ b/docs/source/extending.rst @@ -122,22 +122,24 @@ functions as in the second example. Extending Profile Parsers -------------------------- -The ``ProfileParser`` class is located in the ``diffpy.srfit.fitbase.parser`` -module. The purpose of this class is to read data and metadata from a file or -string and pass those data and metadata to a ``Profile`` instance. The -``Profile`` in turn will pass this information to a ``ProfileGenerator``. +The ``ProfileParser`` class is located in the +``diffpy.srfit.fitbase.profileparser`` module. The purpose of this class is +to read data and metadata from a file or string and pass those data and +metadata to a ``Profile`` instance. The ``Profile`` in turn will pass this +information to a ``ProfileGenerator``. The simplest way to extend the ``ProfileParser`` is to derive a new class from -``ProfileParser`` and overload the ``parseString`` method. By default, the -``parseFile`` method can read an ASCII file and passes the loaded string to the -``parseString`` method. For non-ASCII data one should overload both of these -methods. An example of a customized ``ProfileParser`` is the ``PDFParser`` -class in the ``diffpy.srfit.pdf.pdfparser`` module. +``ProfileParser`` and overload the ``parse_string`` method. By default, the +``parse_file`` method reads generic column data, but once a subclass overloads +``parse_string`` it reads an ASCII file instead and passes the loaded string +to ``parse_string``. For non-ASCII data one should overload ``parse_file`` as +well. An example of a customized ``ProfileParser`` is the ``PDFParser`` class +in the ``diffpy.srfit.pdf.pdfparser`` module. Here is a simple example demonstrating how to extract (x,y) data from a two-column string. :: - def parseString(self, datastring): + def parse_string(self, datastring): xvals = [] yvals = [] diff --git a/src/diffpy/srfit/fitbase/profileparser.py b/src/diffpy/srfit/fitbase/profileparser.py index 78dc3c27..da63dfd7 100644 --- a/src/diffpy/srfit/fitbase/profileparser.py +++ b/src/diffpy/srfit/fitbase/profileparser.py @@ -31,19 +31,22 @@ from diffpy.utils.parsers import load_data removal_verison = "4.0.0" -pdfparser_base = "diffpy.srfit.pdf.pdfparser.PDFParser" -new_base = "diffpy.srfit.fitbase.ProfileParser" +pp_base = "diffpy.srfit.fitbase.profileparser.ProfileParser" parseFile_dep_msg = build_deprecation_message( - pdfparser_base, + pp_base, "parseFile", "parse_file", removal_verison, - new_base=new_base, ) -pp_base = "diffpy.srfit.fitbase.profileparser.ProfileParser" +parseString_dep_msg = build_deprecation_message( + pp_base, + "parseString", + "parse_string", + removal_verison, +) getNumBanks_dep_msg = build_deprecation_message( pp_base, @@ -139,10 +142,14 @@ def getFormat(self): """Get the format string.""" return self._format - def parseString(self, patstring): + def parse_string(self, patstring): """Parse a string and set the _x, _y, _dx, _dy and _meta variables. + Override this in a subclass to add support for a specific data + format. When overridden, `parse_file` dispatches to this method + instead of reading the file as generic column data. + When _dx or _dy cannot be obtained in the data format it is set to None. @@ -159,43 +166,17 @@ def parseString(self, patstring): """ raise NotImplementedError() - # remove parseString too when this file is removed. - @deprecated(parseFile_dep_msg) - def parseFile(self, filename): - """Parse a file and set the _x, _y, _dx, _dy and _meta - variables. - - This wipes out the currently loaded data and selected bank number. - - Parameters - ---------- - filename - The name of the file to parse - - Raises - ---------- - IOError - if the file cannot be read - ParseError - if the file cannot be parsed - """ - infile = open(filename, "r") - self._banks = [] - self._meta = {} - filestring = infile.read() - self.parseString(filestring) - infile.close() - self._meta["filename"] = filename + def parse_file(self, filename, column_format=None): + """Parse a data file to extract data and metadata. - if len(self._banks) < 1: - raise ParseError("There are no data in the banks") + If a subclass overrides `parse_string`, e.g. + `diffpy.srfit.pdf.pdfparser.PDFParser`, this reads the file as + text and dispatches to it to perform format-specific parsing, + including extracting metadata such as `stype`, `qmax` and + `qdamp`. Subclasses may append more than one bank this way. - self.select_bank(0) - return - - def parse_file(self, filename, column_format=None): - """Parse a data file to extract data and metadata, with - automatic handling of uncertainties. + Otherwise this reads generic column data, with automatic handling + of uncertainties, and always produces a single bank: - For files with 2 columns: assumes (x, y) and sets dx, dy to 0. - For files with 3 columns: assumes (x, y, dy) and sets dx to 0. @@ -212,7 +193,10 @@ def parse_file(self, filename, column_format=None): filename : str or Path The name of the file to parse. column_format : tuple of str, optional - The order in which columns appear in the file. + The order in which columns appear in the file. Only used for + generic column data; raises `ParseError` if given for a + parser that overrides `parse_string`, since such parsers + determine the column layout from the file format. If None, the format is auto-detected based on the number of columns. @@ -227,9 +211,43 @@ def parse_file(self, filename, column_format=None): Raises ------ + IOError + if the file cannot be read ParseError - If parsing fails or ambiguity detected. + if parsing fails, ambiguity is detected, or column_format is + given for a format-specific parser """ + if self._has_format_parser(): + if column_format is not None: + raise ParseError( + f"{type(self).__name__} determines the column layout " + "from the file format, so 'column_format' is not " + "supported." + ) + return self._parse_file_via_string(filename) + return self._parse_columns(filename, column_format) + + def _has_format_parser(self): + """Return True if a subclass overrides `parse_string`.""" + return type(self).parse_string is not ProfileParser.parse_string + + def _parse_file_via_string(self, filename): + """Read a file as text and dispatch to `parse_string`.""" + with open(filename, "r") as infile: + filestring = infile.read() + self._banks = [] + self._meta = {} + self.parse_string(filestring) + self._meta["filename"] = str(filename) + + if len(self._banks) < 1: + raise ParseError("There are no data in the banks") + + self.select_bank(0) + return + + def _parse_columns(self, filename, column_format=None): + """Read generic column data.""" # Reset internal state self._banks = [] if isinstance(filename, Path): @@ -251,6 +269,25 @@ def parse_file(self, filename, column_format=None): self._meta["nbanks"] = 1 self.select_bank(0) + @deprecated(parseFile_dep_msg) + def parseFile(self, filename): + """This function is deprecated and will be removed in version + 4.0.0. + + Please use diffpy.srfit.fitbase.ProfileParser.parse_file instead. + """ + return self.parse_file(filename) + + @deprecated(parseString_dep_msg) + def parseString(self, patstring): + """This function is deprecated and will be removed in version + 4.0.0. + + Please use diffpy.srfit.fitbase.ProfileParser.parse_string + instead. + """ + return self.parse_string(patstring) + def _load_file(self, filename): """Load metadata and numeric data from a file.""" meta = load_data(filename, headers=True) diff --git a/src/diffpy/srfit/pdf/pdfparser.py b/src/diffpy/srfit/pdf/pdfparser.py index b45e6b08..ce91b22c 100644 --- a/src/diffpy/srfit/pdf/pdfparser.py +++ b/src/diffpy/srfit/pdf/pdfparser.py @@ -28,19 +28,6 @@ from diffpy.srfit.exceptions import ParseError from diffpy.srfit.fitbase.profileparser import ProfileParser -from diffpy.utils._deprecator import build_deprecation_message, deprecated - -removal_verison = "4.0.0" -base = "diffpy.srfit.pdf.pdfparser.PDFParser" -new_base = "diffpy.srfit.fitbase.ProfileParser" - -parseFile_dep_msg = build_deprecation_message( - base, - "parseFile", - "parse_file", - removal_version=removal_verison, - new_base=new_base, -) class PDFParser(ProfileParser): @@ -119,12 +106,7 @@ class PDFParser(ProfileParser): _format = "PDF" - # Marking this function as deprecated because PDFParser.parseFile calls it - # so when people use PDFParser.parseFile, they will get a - # warning that it is deprecated and they should use - # ProfileParser.parse_file instead. - @deprecated(parseFile_dep_msg) - def parseString(self, patstring): + def parse_string(self, patstring): """Parse a string and set the _x, _y, _dx, _dy and _meta variables. diff --git a/tests/test_pdf.py b/tests/test_pdf.py index 315f6dff..3a97aafa 100644 --- a/tests/test_pdf.py +++ b/tests/test_pdf.py @@ -17,6 +17,7 @@ import io import pickle import unittest +import warnings from itertools import chain import numpy @@ -34,11 +35,11 @@ def testParser1(datafile): data = datafile("ni-q27r100-neutron.gr") parser = PDFParser() - parser.parseFile(data) + parser.parse_file(data) meta = parser._meta - assert data == meta["filename"] + assert str(data) == meta["filename"] assert 1 == meta["nbanks"] assert "N" == meta["stype"] assert 27 == meta["qmax"] @@ -143,6 +144,87 @@ def testParser2(datafile): return +# PDFParser.parse_file dispatches to its parse_string override to recover +# PDF-specific metadata; ProfileParser.parse_file has no such override and +# reads generic column data instead. These tests pin that boundary. +PDF_METADATA_KEYS = ("stype", "qmin", "qmax", "temperature") + + +def test_parse_file_extracts_pdf_metadata(datafile): + """Recover the PDF-specific metadata through the parse_string + hook.""" + parser = PDFParser() + parser.parse_file(datafile("ni-q27r100-neutron.gr")) + metadata = parser.get_metadata() + expected_metadata = { + "stype": "N", + "qmin": 0.87, + "qmax": 27.0, + "temperature": 300.0, + } + actual_metadata = {key: metadata[key] for key in expected_metadata} + assert actual_metadata == expected_metadata + + +def test_parse_file_emits_no_deprecation_warning(datafile): + """Emit no warning, since parse_file is the supported entry + point.""" + with warnings.catch_warnings(record=True) as record: + warnings.simplefilter("always") + PDFParser().parse_file(datafile("ni-q27r100-neutron.gr")) + actual_warnings = [ + str(warning.message) + for warning in record + if issubclass(warning.category, DeprecationWarning) + ] + expected_warnings = [] + assert actual_warnings == expected_warnings + + +def test_parse_file_does_not_extract_pdf_metadata(datafile): + """Read generic column data without any format-specific parsing.""" + parser = ProfileParser() + parser.parse_file(datafile("ni-q27r100-neutron.gr")) + metadata = parser.get_metadata() + actual_pdf_keys = sorted( + key for key in PDF_METADATA_KEYS if key in metadata + ) + expected_pdf_keys = [] + assert actual_pdf_keys == expected_pdf_keys + + +def test_parseFile_deprecated(datafile): + """Deprecated parseFile should still work but emit a + DeprecationWarning and delegate to parse_file.""" + data = datafile("ni-q27r100-neutron.gr") + parser = PDFParser() + with pytest.deprecated_call(): + parser.parseFile(data) + actual_metadata = parser.get_metadata() + + expected_parser = PDFParser() + expected_parser.parse_file(data) + expected_metadata = expected_parser.get_metadata() + + assert actual_metadata == expected_metadata + + +def test_parseString_deprecated(datafile): + """Deprecated parseString should still work but emit a + DeprecationWarning and delegate to parse_string.""" + text = datafile("ni-q27r100-neutron.gr").read_text() + parser = PDFParser() + with pytest.deprecated_call(): + parser.parseString(text) + actual_metadata = parser.get_metadata() + + expected_parser = PDFParser() + expected_parser.parse_string(text) + expected_metadata = expected_parser.get_metadata() + + assert actual_metadata == expected_metadata + + def testGenerator(diffpy_srreal_available, datafile): if not diffpy_srreal_available: pytest.skip("diffpy.srreal package not available") From a466556d1d0b4d649f61a7c7cf2905947192cd4a Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Mon, 3 Aug 2026 11:08:07 -0400 Subject: [PATCH 04/11] fix: use format-specific PDF parsing in PDFContribution.loadData --- src/diffpy/srfit/pdf/pdfcontribution.py | 5 +++-- tests/test_pdf.py | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/src/diffpy/srfit/pdf/pdfcontribution.py b/src/diffpy/srfit/pdf/pdfcontribution.py index 1b22c1fd..4daa7f3c 100644 --- a/src/diffpy/srfit/pdf/pdfcontribution.py +++ b/src/diffpy/srfit/pdf/pdfcontribution.py @@ -20,7 +20,8 @@ __all__ = ["PDFContribution"] -from diffpy.srfit.fitbase import FitContribution, Profile, ProfileParser +from diffpy.srfit.fitbase import FitContribution, Profile +from diffpy.srfit.pdf.pdfparser import PDFParser class PDFContribution(FitContribution): @@ -113,7 +114,7 @@ def loadData(self, datafile): data : str or Path The path to the data file. """ - parser = ProfileParser() + parser = PDFParser() parser.parse_file(datafile) # Pass it to the profile diff --git a/tests/test_pdf.py b/tests/test_pdf.py index 3a97aafa..565076cf 100644 --- a/tests/test_pdf.py +++ b/tests/test_pdf.py @@ -225,6 +225,24 @@ def test_parseString_deprecated(datafile): assert actual_metadata == expected_metadata +def test_loadData_preserves_pdf_metadata(datafile): + """Keep the metadata that configures the PDF calculator. + + Parsing with ProfileParser.parse_file instead of PDFParser dropped + the keys BasePDFGenerator._process_metadata reads. This test needs + no diffpy.srreal so that it runs rather than skips. + """ + contribution = PDFContribution("pdf") + contribution.loadData(datafile("ni-q27r100-neutron.gr")) + actual_metadata = { + "stype": contribution.profile.meta["stype"], + "temperature": contribution.profile.meta["temperature"], + "qmax": contribution.getQmax(), + } + expected_metadata = {"stype": "N", "temperature": 300.0, "qmax": 27.0} + assert actual_metadata == expected_metadata + + def testGenerator(diffpy_srreal_available, datafile): if not diffpy_srreal_available: pytest.skip("diffpy.srreal package not available") From e542ad980852bf11233d6ca8a368a4ff502a2df9 Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Mon, 3 Aug 2026 11:08:20 -0400 Subject: [PATCH 05/11] chore: add news entry for parseFile/parse_file merge --- news/merge-parsefile-parse-file.rst | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) create mode 100644 news/merge-parsefile-parse-file.rst diff --git a/news/merge-parsefile-parse-file.rst b/news/merge-parsefile-parse-file.rst new file mode 100644 index 00000000..a6c7fcc8 --- /dev/null +++ b/news/merge-parsefile-parse-file.rst @@ -0,0 +1,23 @@ +**Added:** + +* + +**Changed:** + +* Change ``ProfileParser.parseFile`` to ``ProfileParser.parse_file``. Now, it dispatches to a subclass's ``parse_string`` override to perform format-specific parsing when one is provided (as ``PDFParser`` does), and otherwise reads generic column data. + +**Deprecated:** + +* Deprecate ``parseFile`` and ``parseString`` in ``ProfileParser``. Use ``parse_file`` and ``parse_string`` instead. + +**Removed:** + +* + +**Fixed:** + +* Fix ``PDFContribution.loadData`` discarding PDF metadata such as ``stype``, ``qmax``, ``qdamp`` and ``temperature``. It parsed with generic column data instead of dispatching to ``PDFParser``'s format-specific parsing, so ``BasePDFGenerator._process_metadata`` had nothing to configure the calculator with. + +**Security:** + +* From 623fc95da9f9c5e2c9a73d4a20658d3afdff60cb Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Mon, 3 Aug 2026 16:47:10 -0400 Subject: [PATCH 06/11] chore: tighten up tests written by claude --- tests/test_pdf.py | 87 +++++++++++++++---------------------- tests/test_profileparser.py | 22 ++++++++++ 2 files changed, 56 insertions(+), 53 deletions(-) diff --git a/tests/test_pdf.py b/tests/test_pdf.py index 565076cf..9d0146b1 100644 --- a/tests/test_pdf.py +++ b/tests/test_pdf.py @@ -17,7 +17,6 @@ import io import pickle import unittest -import warnings from itertools import chain import numpy @@ -147,91 +146,73 @@ def testParser2(datafile): # PDFParser.parse_file dispatches to its parse_string override to recover # PDF-specific metadata; ProfileParser.parse_file has no such override and # reads generic column data instead. These tests pin that boundary. -PDF_METADATA_KEYS = ("stype", "qmin", "qmax", "temperature") def test_parse_file_extracts_pdf_metadata(datafile): - """Recover the PDF-specific metadata through the parse_string - hook.""" + # PDFParser.parse_file is designed to extract PDF metadata from the + # file header. + # Expected: parser.get_metadata() returns a dictionary with + # the expected keys and values. parser = PDFParser() parser.parse_file(datafile("ni-q27r100-neutron.gr")) - metadata = parser.get_metadata() + actual_metadata = parser.get_metadata() expected_metadata = { "stype": "N", "qmin": 0.87, "qmax": 27.0, "temperature": 300.0, + "filename": str(datafile("ni-q27r100-neutron.gr")), + "bank": 0, + "nbanks": 1, } - actual_metadata = {key: metadata[key] for key in expected_metadata} assert actual_metadata == expected_metadata -def test_parse_file_emits_no_deprecation_warning(datafile): - """Emit no warning, since parse_file is the supported entry - point.""" - with warnings.catch_warnings(record=True) as record: - warnings.simplefilter("always") - PDFParser().parse_file(datafile("ni-q27r100-neutron.gr")) - actual_warnings = [ - str(warning.message) - for warning in record - if issubclass(warning.category, DeprecationWarning) - ] - expected_warnings = [] - assert actual_warnings == expected_warnings - - -def test_parse_file_does_not_extract_pdf_metadata(datafile): - """Read generic column data without any format-specific parsing.""" - parser = ProfileParser() - parser.parse_file(datafile("ni-q27r100-neutron.gr")) - metadata = parser.get_metadata() - actual_pdf_keys = sorted( - key for key in PDF_METADATA_KEYS if key in metadata - ) - expected_pdf_keys = [] - assert actual_pdf_keys == expected_pdf_keys - - def test_parseFile_deprecated(datafile): - """Deprecated parseFile should still work but emit a - DeprecationWarning and delegate to parse_file.""" + # Deprecated parseFile should still work but emit a + # DeprecationWarning and delegate to parse_file. + # Expected: parser.get_metadata() returns the same + # dictionary as parse_file. data = datafile("ni-q27r100-neutron.gr") - parser = PDFParser() + deprecated_parser = PDFParser() + # Assert that a DeprecationWarning is raised when calling parseFile. with pytest.deprecated_call(): - parser.parseFile(data) - actual_metadata = parser.get_metadata() + deprecated_parser.parseFile(data) + actual_deprecated_metadata = deprecated_parser.get_metadata() expected_parser = PDFParser() expected_parser.parse_file(data) expected_metadata = expected_parser.get_metadata() - - assert actual_metadata == expected_metadata + # Assert the deprecated parseFile method returns the + # same metadata as the parse_file method. + assert actual_deprecated_metadata == expected_metadata def test_parseString_deprecated(datafile): - """Deprecated parseString should still work but emit a - DeprecationWarning and delegate to parse_string.""" + # Deprecated parseString should still work but emit a + # DeprecationWarning and delegate to parse_string. + # Expected: parser.get_metadata() returns the same + # dictionary as parse_string. text = datafile("ni-q27r100-neutron.gr").read_text() - parser = PDFParser() + deprecated_parser = PDFParser() + # Assert that a DeprecationWarning is raised when calling parseString. with pytest.deprecated_call(): - parser.parseString(text) - actual_metadata = parser.get_metadata() + deprecated_parser.parseString(text) + actual_deprecated_metadata = deprecated_parser.get_metadata() expected_parser = PDFParser() expected_parser.parse_string(text) expected_metadata = expected_parser.get_metadata() - - assert actual_metadata == expected_metadata + # Assert the deprecated parseString method returns the + # same metadata as the parse_string method. + assert actual_deprecated_metadata == expected_metadata def test_loadData_preserves_pdf_metadata(datafile): - """Keep the metadata that configures the PDF calculator. - - Parsing with ProfileParser.parse_file instead of PDFParser dropped - the keys BasePDFGenerator._process_metadata reads. This test needs - no diffpy.srreal so that it runs rather than skips. - """ + # PDFContribution.loadData should preserve PDF metadata from the + # file header. + # Expected: PDFContribution.profile.meta contains the expected + # keys and values. contribution = PDFContribution("pdf") contribution.loadData(datafile("ni-q27r100-neutron.gr")) actual_metadata = { diff --git a/tests/test_profileparser.py b/tests/test_profileparser.py index e1676b15..a41dbde9 100644 --- a/tests/test_profileparser.py +++ b/tests/test_profileparser.py @@ -198,3 +198,25 @@ def test_parse_file_bad(parser_datafiles, input_file, column_order, msg): parser = ProfileParser() with pytest.raises(ParseError, match=re.escape(msg)): parser.parse_file(parser_datafiles / input_file, column_order) + + +def test_parse_file_does_not_extract_pdf_metadata(datafile): + # ProfileParser.parse_file extracts metadata by in the header, + # using "=" to separate keys and values. + # Expected: parser.get_metadata() parses treturns a dict + parser = ProfileParser() + # SAS data is used because the header contains metadata separated by '='. + parser.parse_file(datafile("sas_ellipsoid_testdata.txt")) + actual_metadata = parser.get_metadata() + expected_metadata = { + "pythonclass": "EllipsoidModel", + "scale": 1.0, + "radius_a": 20.0, + "radius_b": 400.0, + "contrast": 3e-06, + "background": 0.01, + "filename": str(datafile("sas_ellipsoid_testdata.txt")), + "nbanks": 1, + "bank": 0, + } + assert actual_metadata == expected_metadata From 5c5ac409390896c4c46db596ffd58f9b3bceb78d Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Mon, 3 Aug 2026 17:22:51 -0400 Subject: [PATCH 07/11] test: add tests for new Parser classes bad cases --- tests/test_profileparser.py | 59 +++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/tests/test_profileparser.py b/tests/test_profileparser.py index a41dbde9..cd130cf8 100644 --- a/tests/test_profileparser.py +++ b/tests/test_profileparser.py @@ -35,6 +35,15 @@ # UC11: User loads file with x, y, dx, and dy but specifies column_format with # duplicate values # expected: ParseError is raised +# UC13: User loads file with only dx, dy columns specified in column_format +# (neither x nor y is present) +# expected: ParseError is raised +# UC14: User loads a file with a format-specific parser (parse_string +# overridden, as PDFParser does) and also specifies column_format +# expected: ParseError is raised +# UC15: User loads a file with a format-specific parser whose parse_string +# does not populate any banks +# expected: ParseError is raised EXPECTED_META = { "wavelength": 0.1, @@ -192,6 +201,13 @@ def test_parse_file( "column_format contains invalid label 'invalid'. " "Valid labels are 'x', 'y', 'dx', and 'dy'.", ), + # UC13: column_format specifies dx, dy but neither x nor y + # expected: ParseError is raised + ( + "two_col.txt", + ("dx", "dy"), + "Both 'x' and 'y' columns must be present in the data.", + ), ], ) def test_parse_file_bad(parser_datafiles, input_file, column_order, msg): @@ -200,6 +216,49 @@ def test_parse_file_bad(parser_datafiles, input_file, column_order, msg): parser.parse_file(parser_datafiles / input_file, column_order) +class _FormatSpecificParser(ProfileParser): + """A parser with a format-specific parse_string override, mimicking + PDFParser, used to test parse_file's dispatch logic.""" + + def parse_string(self, patstring): + raise NotImplementedError + + +class _NoBanksParser(ProfileParser): + """A parser whose parse_string never populates any banks.""" + + def parse_string(self, patstring): + pass + + +def test_parse_file_bad_parser( + parser_datafiles, +): + # UC14: column_format is specified for a parser that overrides + # parse_string, so it determines the column layout from the file + # format instead. + # expected: ParseError is raised + parser = _FormatSpecificParser() + expected_msg = ( + "_FormatSpecificParser determines the column layout from the " + "file format, so 'column_format' is not supported." + ) + with pytest.raises(ParseError, match=re.escape(expected_msg)): + parser.parse_file( + parser_datafiles / "four_col.gr", ("x", "y", "dx", "dy") + ) + + +def test_parse_file_bad_no_banks_parsed(parser_datafiles): + # UC15: A format-specific parser's parse_string does not populate any + # banks. + # expected: ParseError is raised + parser = _NoBanksParser() + expected_msg = "There are no data in the banks" + with pytest.raises(ParseError, match=re.escape(expected_msg)): + parser.parse_file(parser_datafiles / "four_col.gr") + + def test_parse_file_does_not_extract_pdf_metadata(datafile): # ProfileParser.parse_file extracts metadata by in the header, # using "=" to separate keys and values. From e091a82c0acabe7bf2156d3ffe0e85228a26c734 Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Mon, 3 Aug 2026 17:28:11 -0400 Subject: [PATCH 08/11] rm unnecessary blank lines --- src/diffpy/srfit/fitbase/profileparser.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/diffpy/srfit/fitbase/profileparser.py b/src/diffpy/srfit/fitbase/profileparser.py index da63dfd7..ee0a0ced 100644 --- a/src/diffpy/srfit/fitbase/profileparser.py +++ b/src/diffpy/srfit/fitbase/profileparser.py @@ -239,10 +239,8 @@ def _parse_file_via_string(self, filename): self._meta = {} self.parse_string(filestring) self._meta["filename"] = str(filename) - if len(self._banks) < 1: raise ParseError("There are no data in the banks") - self.select_bank(0) return @@ -302,7 +300,6 @@ def _load_file(self, filename): def _detect_column_format(self, data, column_format): """Auto-detect or validate column format.""" num_cols = data.shape[1] - if column_format is None: if num_cols == 2: column_format = ("x", "y") @@ -334,12 +331,10 @@ def _map_column_labels_to_data(self, data, column_format): columns = {} for i, label in enumerate(column_format): columns[label] = data[:, i] - if "x" not in columns or "y" not in columns: raise ParseError( "Both 'x' and 'y' columns must be present in the data." ) - return columns @staticmethod @@ -389,17 +384,13 @@ def select_bank(self, index): """ if index is None: index = self._meta.get("bank", 0) - numbanks = self.get_num_banks() if index > numbanks: raise IndexError("Bank index out of range") - if index < 0: index += numbanks - if index < 0: raise IndexError("Bank index out of range") - self._meta["bank"] = index self._meta["nbanks"] = numbanks self._x, self._y, self._dx, self._dy = self._banks[index] From 3d547ddb7cd969590aeb0fc0233728fc9fd681df Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Mon, 3 Aug 2026 21:56:06 -0400 Subject: [PATCH 09/11] parameterize bad parser testing --- tests/test_profileparser.py | 48 +++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/tests/test_profileparser.py b/tests/test_profileparser.py index cd130cf8..85835f3b 100644 --- a/tests/test_profileparser.py +++ b/tests/test_profileparser.py @@ -231,32 +231,34 @@ def parse_string(self, patstring): pass +@pytest.mark.parametrize( + "parser, column_format, expected_msg", + [ + # C1: column_format is specified for a parser that overrides + # parse_string, so it determines the column layout from the file + # format instead. + # Expected: ParseError is raised. + ( + _FormatSpecificParser(), + ("x", "y", "dx", "dy"), + "_FormatSpecificParser determines the column layout from the " + "file format, so 'column_format' is not supported.", + ), + # C2: A format-specific parser's parse_string does not populate + # any banks. + # Expected: ParseError is raised. + ( + _NoBanksParser(), + None, + "There are no data in the banks", + ), + ], +) def test_parse_file_bad_parser( - parser_datafiles, + parser_datafiles, parser, column_format, expected_msg ): - # UC14: column_format is specified for a parser that overrides - # parse_string, so it determines the column layout from the file - # format instead. - # expected: ParseError is raised - parser = _FormatSpecificParser() - expected_msg = ( - "_FormatSpecificParser determines the column layout from the " - "file format, so 'column_format' is not supported." - ) - with pytest.raises(ParseError, match=re.escape(expected_msg)): - parser.parse_file( - parser_datafiles / "four_col.gr", ("x", "y", "dx", "dy") - ) - - -def test_parse_file_bad_no_banks_parsed(parser_datafiles): - # UC15: A format-specific parser's parse_string does not populate any - # banks. - # expected: ParseError is raised - parser = _NoBanksParser() - expected_msg = "There are no data in the banks" with pytest.raises(ParseError, match=re.escape(expected_msg)): - parser.parse_file(parser_datafiles / "four_col.gr") + parser.parse_file(parser_datafiles / "four_col.gr", column_format) def test_parse_file_does_not_extract_pdf_metadata(datafile): From 0965c013ea99a95baaab5ee9b0e4c4d5b5140529 Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Mon, 3 Aug 2026 22:06:15 -0400 Subject: [PATCH 10/11] parameterize test on the deprecated methods --- tests/test_pdf.py | 63 +++++++++++++++++++++++------------------------ 1 file changed, 31 insertions(+), 32 deletions(-) diff --git a/tests/test_pdf.py b/tests/test_pdf.py index 9d0146b1..90e0545a 100644 --- a/tests/test_pdf.py +++ b/tests/test_pdf.py @@ -168,44 +168,43 @@ def test_parse_file_extracts_pdf_metadata(datafile): assert actual_metadata == expected_metadata -def test_parseFile_deprecated(datafile): - # Deprecated parseFile should still work but emit a - # DeprecationWarning and delegate to parse_file. - # Expected: parser.get_metadata() returns the same - # dictionary as parse_file. - data = datafile("ni-q27r100-neutron.gr") - deprecated_parser = PDFParser() - # Assert that a DeprecationWarning is raised when calling parseFile. - with pytest.deprecated_call(): - deprecated_parser.parseFile(data) - actual_deprecated_metadata = deprecated_parser.get_metadata() - - expected_parser = PDFParser() - expected_parser.parse_file(data) - expected_metadata = expected_parser.get_metadata() - # Assert the deprecated parseFile method returns the - # same metadata as the parse_file method. - assert actual_deprecated_metadata == expected_metadata - +@pytest.mark.parametrize( + ("deprecated_method", "current_method", "prepare_arg"), + [ + # C1: parseFile is deprecated in favor of parse_file. + # Expected: parseFile still parses the file but emits a + # DeprecationWarning and returns the same metadata as parse_file. + ( + "parseFile", + "parse_file", + lambda datafile: datafile("ni-q27r100-neutron.gr"), + ), + # C2: parseString is deprecated in favor of parse_string. + # Expected: parseString still parses the text but emits a + # DeprecationWarning and returns the same metadata as + # parse_string. + ( + "parseString", + "parse_string", + lambda datafile: datafile("ni-q27r100-neutron.gr").read_text(), + ), + ], +) +def test_deprecated_parse_methods_delegate( + deprecated_method, current_method, prepare_arg, datafile +): + arg = prepare_arg(datafile) -def test_parseString_deprecated(datafile): - # Deprecated parseString should still work but emit a - # DeprecationWarning and delegate to parse_string. - # Expected: parser.get_metadata() returns the same - # dictionary as parse_string. - text = datafile("ni-q27r100-neutron.gr").read_text() deprecated_parser = PDFParser() - # Assert that a DeprecationWarning is raised when calling parseString. with pytest.deprecated_call(): - deprecated_parser.parseString(text) - actual_deprecated_metadata = deprecated_parser.get_metadata() + getattr(deprecated_parser, deprecated_method)(arg) + actual_metadata = deprecated_parser.get_metadata() expected_parser = PDFParser() - expected_parser.parse_string(text) + getattr(expected_parser, current_method)(arg) expected_metadata = expected_parser.get_metadata() - # Assert the deprecated parseString method returns the - # same metadata as the parse_string method. - assert actual_deprecated_metadata == expected_metadata + + assert actual_metadata == expected_metadata def test_loadData_preserves_pdf_metadata(datafile): From eb701672c9394546b80680f38b7943eb00c94dcc Mon Sep 17 00:00:00 2001 From: Caden Myers Date: Tue, 4 Aug 2026 08:29:53 -0400 Subject: [PATCH 11/11] rm some more whitespace --- tests/test_pdf.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/test_pdf.py b/tests/test_pdf.py index 90e0545a..b8d09aa2 100644 --- a/tests/test_pdf.py +++ b/tests/test_pdf.py @@ -194,16 +194,13 @@ def test_deprecated_parse_methods_delegate( deprecated_method, current_method, prepare_arg, datafile ): arg = prepare_arg(datafile) - deprecated_parser = PDFParser() with pytest.deprecated_call(): getattr(deprecated_parser, deprecated_method)(arg) actual_metadata = deprecated_parser.get_metadata() - expected_parser = PDFParser() getattr(expected_parser, current_method)(arg) expected_metadata = expected_parser.get_metadata() - assert actual_metadata == expected_metadata