Merge pull request #360 from lsst/tickets/DM-40032

DM-40032: Add missing stacklevel param and remove deprecated code

doc/changes/DM-40032.removal.rst

@@ -0,0 +1 @@
+Removed support for reading quantum graphs in pickle format.

pyproject.toml

@@ -4,6 +4,7 @@ build-backend = "setuptools.build_meta"
 
 [project]
 name = "lsst-pipe-base"
+requires-python = ">=3.10.0"
 description = "Pipeline infrastructure for the Rubin Science Pipelines."
 license = {text = "GPLv3+ License"}
 readme = "README.md"

python/lsst/pipe/base/_datasetQueryConstraints.py

@@ -31,6 +31,8 @@
 from collections.abc import Iterable, Iterator
 from typing import Protocol
 
+from lsst.utils.introspection import find_outside_stacklevel
+
 
 class DatasetQueryConstraintVariant(Iterable, Protocol):
     """Base for all the valid variants for controlling
@@ -83,7 +85,11 @@ def fromExpression(cls, expression: str) -> "DatasetQueryConstraintVariant":
             return cls.OFF
         else:
             if " " in expression:
-                warnings.warn("Whitespace found in expression will be trimmed", RuntimeWarning)
+                warnings.warn(
+                    "Whitespace found in expression will be trimmed",
+                    RuntimeWarning,
+                    stacklevel=find_outside_stacklevel("lsst.pipe.base"),
+                )
                 expression = expression.replace(" ", "")
             members = expression.split(",")
             return cls.LIST(members)

python/lsst/pipe/base/_task_metadata.py

@@ -28,11 +28,9 @@
 from typing import Any, Protocol
 
 from lsst.daf.butler._compat import _BaseModelCompat
+from lsst.utils.introspection import find_outside_stacklevel
 from pydantic import Field, StrictBool, StrictFloat, StrictInt, StrictStr
 
-_DEPRECATION_REASON = "Will be removed after v25."
-_DEPRECATION_VERSION = "v24"
-
 # The types allowed in a Task metadata field are restricted
 # to allow predictable serialization.
 _ALLOWED_PRIMITIVE_TYPES = (str, float, int, bool)
@@ -256,41 +254,41 @@ def getArray(self, key: str) -> list[Any]:
             # Report the correct key.
             raise KeyError(f"'{key}' not found") from None
 
-    def names(self, topLevelOnly: bool = True) -> set[str]:
+    def names(self, topLevelOnly: bool | None = None) -> set[str]:
         """Return the hierarchical keys from the metadata.
 
         Parameters
         ----------
-        topLevelOnly  : `bool`
-            If true, return top-level keys, otherwise full metadata item keys.
+        topLevelOnly  : `bool` or `None`, optional
+            This parameter is deprecated and will be removed in the future.
+            If given it can only be `False`.  All names in the hierarchy are
+            always returned.
 
         Returns
         -------
         names : `collections.abc.Set`
-            A set of top-level keys or full metadata item keys, including
-            the top-level keys.
-
-        Notes
-        -----
-        Should never be called in new code with ``topLevelOnly`` set to `True`
-        -- this is equivalent to asking for the keys and is the default
-        when iterating through the task metadata. In this case a deprecation
-        message will be issued and the ability will raise an exception
-        in a future release.
-
-        When ``topLevelOnly`` is `False` all keys, including those from the
-        hierarchy and the top-level hierarchy, are returned.
+            A set of all keys, including those from the hierarchy and the
+            top-level hierarchy.
         """
         if topLevelOnly:
-            warnings.warn("Use keys() instead. " + _DEPRECATION_REASON, FutureWarning)
-            return set(self.keys())
-        else:
-            names = set()
-            for k, v in self.items():
-                names.add(k)  # Always include the current level
-                if isinstance(v, TaskMetadata):
-                    names.update({k + "." + item for item in v.names(topLevelOnly=topLevelOnly)})
-            return names
+            raise RuntimeError(
+                "The topLevelOnly parameter is no longer supported and can not have a True value."
+            )
+
+        if topLevelOnly is False:
+            warnings.warn(
+                "The topLevelOnly parameter is deprecated and is always assumed to be False."
+                " It will be removed completely after v26.",
+                category=FutureWarning,
+                stacklevel=find_outside_stacklevel("lsst.pipe.base"),
+            )
+
+        names = set()
+        for k, v in self.items():
+            names.add(k)  # Always include the current level
+            if isinstance(v, TaskMetadata):
+                names.update({k + "." + item for item in v.names()})
+        return names
 
     def paramNames(self, topLevelOnly: bool) -> set[str]:
         """Return hierarchical names.

python/lsst/pipe/base/graph/graph.py

@@ -26,11 +26,9 @@
 import json
 import lzma
 import os
-import pickle
 import struct
 import time
 import uuid
-import warnings
 from collections import defaultdict, deque
 from collections.abc import Generator, Iterable, Mapping, MutableMapping
 from itertools import chain
@@ -901,9 +899,7 @@ def loadUri(
         uri : convertible to `~lsst.resources.ResourcePath`
             URI from where to load the graph.
         universe : `~lsst.daf.butler.DimensionUniverse`, optional
-            `~lsst.daf.butler.DimensionUniverse` instance, not used by the
-            method itself but needed to ensure that registry data structures
-            are initialized. If `None` it is loaded from the `QuantumGraph`
+            If `None` it is loaded from the `QuantumGraph`
             saved structure. If supplied, the
             `~lsst.daf.butler.DimensionUniverse` from the loaded `QuantumGraph`
             will be validated against the supplied argument for compatibility.
@@ -929,7 +925,7 @@ def loadUri(
         Raises
         ------
         TypeError
-            Raised if pickle contains instance of a type other than
+            Raised if file contains instance of a type other than
             `QuantumGraph`.
         ValueError
             Raised if one or more of the nodes requested is not in the
@@ -940,33 +936,15 @@ def loadUri(
             Raise if Supplied `~lsst.daf.butler.DimensionUniverse` is not
             compatible with the `~lsst.daf.butler.DimensionUniverse` saved in
             the graph.
-
-        Notes
-        -----
-        Reading Quanta from pickle requires existence of singleton
-        `~lsst.daf.butler.DimensionUniverse` which is usually instantiated
-        during `~lsst.daf.butler.Registry` initialization. To make sure
-        that `~lsst.daf.butler.DimensionUniverse` exists this method
-        accepts dummy `~lsst.daf.butler.DimensionUniverse` argument.
         """
         uri = ResourcePath(uri)
-        # With ResourcePath we have the choice of always using a local file
-        # or reading in the bytes directly. Reading in bytes can be more
-        # efficient for reasonably-sized pickle files when the resource
-        # is remote. For now use the local file variant. For a local file
-        # as_local() does nothing.
-
-        if uri.getExtension() in (".pickle", ".pkl"):
-            with uri.as_local() as local, open(local.ospath, "rb") as fd:
-                warnings.warn("Pickle graphs are deprecated, please re-save your graph with the save method")
-                qgraph = pickle.load(fd)
-        elif uri.getExtension() in (".qgraph"):
+        if uri.getExtension() in {".qgraph"}:
             with LoadHelper(uri, minimumVersion) as loader:
                 qgraph = loader.load(universe, nodes, graphID)
         else:
-            raise ValueError("Only know how to handle files saved as `pickle`, `pkl`, or `qgraph`")
+            raise ValueError(f"Only know how to handle files saved as `.qgraph`, not {uri}")
         if not isinstance(qgraph, QuantumGraph):
-            raise TypeError(f"QuantumGraph save file contains unexpected object type: {type(qgraph)}")
+            raise TypeError(f"QuantumGraph file {uri} contains unexpected object type: {type(qgraph)}")
         return qgraph
 
     @classmethod
@@ -995,17 +973,14 @@ def readHeader(cls, uri: ResourcePathExpression, minimumVersion: int = 3) -> str
         Raises
         ------
         ValueError
-            Raised if `QuantumGraph` was saved as a pickle.
             Raised if the extension of the file specified by uri is not a
             `QuantumGraph` extension.
         """
         uri = ResourcePath(uri)
-        if uri.getExtension() in (".pickle", ".pkl"):
-            raise ValueError("Reading a header from a pickle save is not supported")
-        elif uri.getExtension() in (".qgraph"):
+        if uri.getExtension() in {".qgraph"}:
             return LoadHelper(uri, minimumVersion).readHeader()
         else:
-            raise ValueError("Only know how to handle files saved as `qgraph`")
+            raise ValueError("Only know how to handle files saved as `.qgraph`")
 
     def buildAndPrintHeader(self) -> None:
         """Create a header that would be used in a save of this object and
@@ -1020,7 +995,7 @@ def save(self, file: BinaryIO) -> None:
         Parameters
         ----------
         file : `io.BufferedIOBase`
-            File to write pickle data open in binary mode.
+            File to write data open in binary mode.
         """
         buffer = self._buildSaveObject()
         file.write(buffer)  # type: ignore # Ignore because bytearray is safe to use in place of bytes
@@ -1155,22 +1130,18 @@ def _buildSaveObject(self, returnHeader: bool = False) -> bytearray | tuple[byte
         map_lengths = struct.pack(fmt_string, len(header_encode))
 
         # write each component of the save out in a deterministic order
-        # buffer = io.BytesIO()
-        # buffer.write(map_lengths)
-        # buffer.write(taskDef_pickle)
-        # buffer.write(map_pickle)
         buffer = bytearray()
         buffer.extend(MAGIC_BYTES)
         buffer.extend(save_bytes)
         buffer.extend(map_lengths)
         buffer.extend(header_encode)
-        # Iterate over the length of pickleData, and for each element pop the
+        # Iterate over the length of jsonData, and for each element pop the
         # leftmost element off the deque and write it out. This is to save
         # memory, as the memory is added to the buffer object, it is removed
         # from from the container.
         #
-        # Only this section needs to worry about memory pressue because
-        # everything else written to the buffer prior to this pickle data is
+        # Only this section needs to worry about memory pressure because
+        # everything else written to the buffer prior to this data is
         # only on the order of kilobytes to low numbers of megabytes.
         while jsonData:
             buffer.extend(jsonData.popleft())
@@ -1193,11 +1164,9 @@ def load(
         Parameters
         ----------
         file : `io.IO` of bytes
-            File with pickle data open in binary mode.
+            File with data open in binary mode.
         universe : `~lsst.daf.butler.DimensionUniverse`, optional
-            `~lsst.daf.butler.DimensionUniverse` instance, not used by the
-            method itself but needed to ensure that registry data structures
-            are initialized. If `None` it is loaded from the `QuantumGraph`
+            If `None` it is loaded from the `QuantumGraph`
             saved structure. If supplied, the
             `~lsst.daf.butler.DimensionUniverse` from the loaded `QuantumGraph`
             will be validated against the supplied argument for compatibility.
@@ -1223,32 +1192,18 @@ def load(
         Raises
         ------
         TypeError
-            Raised if pickle contains instance of a type other than
+            Raised if data contains instance of a type other than
             `QuantumGraph`.
         ValueError
             Raised if one or more of the nodes requested is not in the
             `QuantumGraph` or if graphID parameter does not match the graph
             being loaded or if the supplied uri does not point at a valid
             `QuantumGraph` save file.
-
-        Notes
-        -----
-        Reading Quanta from pickle requires existence of singleton
-        `~lsst.daf.butler.DimensionUniverse` which is usually instantiated
-        during `~lsst.daf.butler.Registry` initialization. To make sure that
-        `~lsst.daf.butler.DimensionUniverse` exists this method accepts dummy
-        `~lsst.daf.butler.DimensionUniverse` argument.
         """
-        # Try to see if the file handle contains pickle data, this will be
-        # removed in the future
-        try:
-            qgraph = pickle.load(file)
-            warnings.warn("Pickle graphs are deprecated, please re-save your graph with the save method")
-        except pickle.UnpicklingError:
-            with LoadHelper(file, minimumVersion) as loader:
-                qgraph = loader.load(universe, nodes, graphID)
+        with LoadHelper(file, minimumVersion) as loader:
+            qgraph = loader.load(universe, nodes, graphID)
         if not isinstance(qgraph, QuantumGraph):
-            raise TypeError(f"QuantumGraph pickle file has contains unexpected object type: {type(qgraph)}")
+            raise TypeError(f"QuantumGraph file contains unexpected object type: {type(qgraph)}")
         return qgraph
 
     def iterTaskGraph(self) -> Generator[TaskDef, None, None]:

python/lsst/pipe/base/pipelineIR.py

@@ -43,6 +43,7 @@
 
 import yaml
 from lsst.resources import ResourcePath, ResourcePathExpression
+from lsst.utils.introspection import find_outside_stacklevel
 
 
 class _Tags(enum.Enum):
@@ -315,7 +316,8 @@ def formatted(self, parameters: ParametersIR) -> ConfigIR:
                 warnings.warn(
                     f"config {key} contains value {match.group(0)} which is formatted like a "
                     "Pipeline parameter but was not found within the Pipeline, if this was not "
-                    "intentional, check for a typo"
+                    "intentional, check for a typo",
+                    stacklevel=find_outside_stacklevel("lsst.pipe.base"),
                 )
         return new_config
 

tests/test_task.py

@@ -203,7 +203,7 @@ def testGetFullMetadata(self):
         self.assertIsInstance(fullMetadata["addMult:mult"], _TASK_METADATA_TYPE)
         self.assertEqual(set(fullMetadata), {"addMult", "addMult:add", "addMult:mult"})
 
-        all_names = fullMetadata.names(topLevelOnly=False)
+        all_names = fullMetadata.names()
         self.assertIn("addMult", all_names)
         self.assertIn("addMult.runStartUtc", all_names)
 

tests/test_taskmetadata.py

@@ -72,7 +72,7 @@ def testTaskMetadata(self):
 
         self.assertEqual(meta.paramNames(topLevelOnly=False), {"test", "new.int", "new.str"})
         self.assertEqual(meta.paramNames(topLevelOnly=True), {"test"})
-        self.assertEqual(meta.names(topLevelOnly=False), {"test", "new", "new.int", "new.str"})
+        self.assertEqual(meta.names(), {"test", "new", "new.int", "new.str"})
         self.assertEqual(meta.keys(), ("test", "new"))
         self.assertEqual(len(meta), 2)
         self.assertEqual(len(meta["new"]), 2)
@@ -235,6 +235,14 @@ def testNumpy(self):
         with self.assertRaises(ValueError):
             meta["numpy"] = numpy.zeros(5)
 
+    def test_deprecated(self):
+        meta = TaskMetadata()
+        with self.assertRaises(RuntimeError):
+            meta.names(topLevelOnly=True)
+
+        with self.assertWarns(FutureWarning):
+            meta.names(topLevelOnly=False)
+
 
 if __name__ == "__main__":
     unittest.main()