fix: Fix the hierarchy associated with ProfileParser.parse_file() and PDFParser.parse_file()

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 = []

news/merge-parsefile-parse-file.rst

@@ -0,0 +1,23 @@
+**Added:**
+
+* <news item>
+
+**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:**
+
+* <news item>
+
+**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:**
+
+* <news item>

news/profileparser_dep.rst

@@ -1,20 +1,18 @@
 **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:**
 
 * <news item>
 
 **Deprecated:**
 
-* Deprecate ``PDFParser``. Use ``ProfileParser`` instead.
-* Deprecate ``getNumBank``, ``selectBank``, ``getFormat``, ``getData``, and ``getMetaData`` in ``ProfileParser``.
+* Deprecate ``getNumBanks``, ``selectBank``, ``getData``, and ``getMetaData`` in ``ProfileParser``.
 
 **Removed:**
 

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

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
-
-        if len(self._banks) < 1:
-            raise ParseError("There are no data in the banks")
+    def parse_file(self, filename, column_format=None):
+        """Parse a data file to extract data and metadata.
 
-        self.select_bank(0)
-        return
+        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.
 
-    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,41 @@ 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 +267,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)
@@ -265,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")
@@ -297,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
@@ -352,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]

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

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.