Merge pull request #932 from lsst/tickets/DM-38687

DM-38687: Remove deprecated DimensionPacker factory methods.

doc/changes/DM-38687.removal.md

@@ -0,0 +1,3 @@
+Removed various already-deprecated factory methods for `DimensionPacker` objects and their support code, as well as the concrete `ObservationDimensionPacker`.
+
+While `daf_butler` still defines the `DimensionPacker` abstract interface, all construction logic has moved to downstream packages.

doc/lsst.daf.butler/index.rst

@@ -201,10 +201,6 @@ General utilities
    :no-main-docstr:
    :headings: ^"
 
-.. automodapi:: lsst.daf.butler.instrument
-   :no-main-docstr:
-   :headings: ^"
-
 .. automodapi:: lsst.daf.butler.json
    :no-main-docstr:
    :headings: ^"

python/lsst/daf/butler/__init__.py

@@ -54,7 +54,7 @@
 from ._formatter import *
 from ._labeled_butler_factory import *
 
-# Do not import 'instrument' or 'json' at all by default.
+# Do not import 'json' at all by default.
 from ._limited_butler import *
 from ._location import *
 from ._named import *

python/lsst/daf/butler/dimensions/_config.py

@@ -29,7 +29,6 @@
 
 __all__ = ("DimensionConfig",)
 
-import warnings
 from collections.abc import Iterator, Mapping, Sequence
 from typing import Any
 
@@ -45,7 +44,6 @@
 )
 from ._elements import KeyColumnSpec, MetadataColumnSpec
 from ._governor import GovernorDimensionConstructionVisitor
-from ._packer import DimensionPackerConstructionVisitor
 from ._skypix import SkyPixConstructionVisitor
 from .construction import DimensionConstructionBuilder, DimensionConstructionVisitor
 
@@ -77,8 +75,7 @@ class DimensionConfig(ConfigSubset):
     - topology: a nested dictionary with ``spatial`` and ``temporal`` keys,
       with dictionary values that each define a `StandardTopologicalFamily`.
 
-    - packers: a nested dictionary whose entries define factories for a
-      `DimensionPacker` instance.
+    - packers: ignored.
 
     See the documentation for the linked classes above for more information
     on the configuration syntax.
@@ -267,31 +264,6 @@ def _extractTopologyVisitors(self) -> Iterator[DimensionConstructionVisitor]:
                     members=members,
                 )
 
-    # TODO: remove this method and callers on DM-38687.
-    # Note that the corresponding entries in the dimensions config should
-    # not be removed at that time, because that's formally a schema migration.
-    def _extractPackerVisitors(self) -> Iterator[DimensionConstructionVisitor]:
-        """Process the 'packers' section of the configuration.
-
-        Yields construction visitors for each `DimensionPackerFactory`.
-
-        Yields
-        ------
-        visitor : `DimensionConstructionVisitor`
-            Object that adds a `DinmensionPackerFactory` to an
-            under-construction `DimensionUniverse`.
-        """
-        with warnings.catch_warnings():
-            # Don't warn when deprecated code calls other deprecated code.
-            warnings.simplefilter("ignore", FutureWarning)
-            for name, subconfig in self["packers"].items():
-                yield DimensionPackerConstructionVisitor(
-                    name=name,
-                    clsName=subconfig["cls"],
-                    fixed=subconfig["fixed"],
-                    dimensions=subconfig["dimensions"],
-                )
-
     def makeBuilder(self) -> DimensionConstructionBuilder:
         """Construct a `DinmensionConstructionBuilder`.
 
@@ -313,5 +285,4 @@ def makeBuilder(self) -> DimensionConstructionBuilder:
         builder.update(self._extractSkyPixVisitors())
         builder.update(self._extractElementVisitors())
         builder.update(self._extractTopologyVisitors())
-        builder.update(self._extractPackerVisitors())
         return builder

python/lsst/daf/butler/dimensions/_coordinate.py

@@ -45,7 +45,7 @@
 import warnings
 from abc import abstractmethod
 from collections.abc import Iterable, Iterator, Mapping, Set
-from typing import TYPE_CHECKING, Any, ClassVar, Literal, cast, overload
+from typing import TYPE_CHECKING, Any, ClassVar, cast
 
 from deprecated.sphinx import deprecated
 from lsst.daf.butler._compat import _BaseModelCompat
@@ -874,50 +874,6 @@ def timespan(self) -> Timespan | None:
         else:
             return Timespan.intersection(*timespans)
 
-    @overload
-    def pack(self, name: str, *, returnMaxBits: Literal[True]) -> tuple[int, int]:
-        ...
-
-    @overload
-    def pack(self, name: str, *, returnMaxBits: Literal[False]) -> int:
-        ...
-
-    # TODO: Remove this method and its overloads above on DM-38687.
-    @deprecated(
-        "Deprecated in favor of configurable dimension packers.  Will be removed after v26.",
-        version="v26",
-        category=FutureWarning,
-    )
-    def pack(self, name: str, *, returnMaxBits: bool = False) -> tuple[int, int] | int:
-        """Pack this data ID into an integer.
-
-        Parameters
-        ----------
-        name : `str`
-            Name of the `DimensionPacker` algorithm (as defined in the
-            dimension configuration).
-        returnMaxBits : `bool`, optional
-            If `True` (`False` is default), return the maximum number of
-            nonzero bits in the returned integer across all data IDs.
-
-        Returns
-        -------
-        packed : `int`
-            Integer ID.  This ID is unique only across data IDs that have
-            the same values for the packer's "fixed" dimensions.
-        maxBits : `int`, optional
-            Maximum number of nonzero bits in ``packed``.  Not returned unless
-            ``returnMaxBits`` is `True`.
-
-        Notes
-        -----
-        Accessing this attribute if `hasRecords` returns `False` is a logic
-        error that may or may not raise an exception, depending on the
-        implementation and whether assertions are enabled.
-        """
-        assert self.hasRecords(), "pack() may only be called if hasRecords() returns True."
-        return self.universe.makePacker(name, self).pack(self, returnMaxBits=returnMaxBits)
-
     def to_simple(self, minimal: bool = False) -> SerializedDataCoordinate:
         """Convert this class to a simple python type.
 

python/lsst/daf/butler/dimensions/_packer.py

@@ -29,17 +29,11 @@
 
 __all__ = ("DimensionPacker",)
 
-import warnings
 from abc import ABCMeta, abstractmethod
-from collections.abc import Iterable, Set
 from typing import TYPE_CHECKING, Any
 
-from deprecated.sphinx import deprecated
-from lsst.utils import doImportType
-
 from ._coordinate import DataCoordinate, DataId
 from ._graph import DimensionGraph, DimensionGroup
-from .construction import DimensionConstructionBuilder, DimensionConstructionVisitor
 
 if TYPE_CHECKING:  # Imports needed only for type annotations; may be circular.
     from ._universe import DimensionUniverse
@@ -186,122 +180,3 @@ def unpack(self, packedId: int) -> DataCoordinate:
     The packed ID values are only unique and reversible with these
     dimensions held fixed.
     """
-
-
-# TODO: Remove this class on DM-38687.
-@deprecated(
-    "Deprecated in favor of configurable dimension packers.  Will be removed after v26.",
-    version="v26",
-    category=FutureWarning,
-)
-class DimensionPackerFactory:
-    """A factory class for `DimensionPacker` instances.
-
-    Can be constructed from configuration.
-
-    This class is primarily intended for internal use by `DimensionUniverse`.
-
-    Parameters
-    ----------
-    clsName : `str`
-        Fully-qualified name of the packer class this factory constructs.
-    fixed : `~collections.abc.Set` [ `str` ]
-        Names of dimensions whose values must be provided to the packer when it
-        is constructed.  This will be expanded lazily into a `DimensionGroup`
-        prior to `DimensionPacker` construction.
-    dimensions : `~collections.abc.Set` [ `str` ]
-        Names of dimensions whose values are passed to `DimensionPacker.pack`.
-        This will be expanded lazily into a `DimensionGroup` prior to
-        `DimensionPacker` construction.
-    """
-
-    def __init__(
-        self,
-        clsName: str,
-        fixed: Set[str],
-        dimensions: Set[str],
-    ):
-        # We defer turning these into DimensionGroup objects until first use
-        # because __init__ is called before a DimensionUniverse exists, and
-        # DimensionGroup instances can only be constructed afterwards.
-        self._fixed: Set[str] | DimensionGroup = fixed
-        self._dimensions: Set[str] | DimensionGroup = dimensions
-        self._clsName = clsName
-        self._cls: type[DimensionPacker] | None = None
-
-    def __call__(self, universe: DimensionUniverse, fixed: DataCoordinate) -> DimensionPacker:
-        """Construct a `DimensionPacker` instance for the given fixed data ID.
-
-        Parameters
-        ----------
-        universe : `DimensionUniverse`
-            The dimension universe for which this packer should be created.
-        fixed : `DataCoordinate`
-            Data ID that provides values for the "fixed" dimensions of the
-            packer.  Must be expanded with all metadata known to the
-            `Registry`.  ``fixed.hasRecords()`` must return `True`.
-        """
-        # Construct DimensionGroup instances if necessary on first use.
-        # See related comment in __init__.
-        self._fixed = universe.conform(self._fixed)
-        self._dimensions = universe.conform(self._dimensions)
-        assert fixed.graph.issuperset(self._fixed)
-        if self._cls is None:
-            packer_class = doImportType(self._clsName)
-            assert not isinstance(
-                packer_class, DimensionPacker
-            ), f"Packer class {self._clsName} must be a DimensionPacker."
-            self._cls = packer_class
-        return self._cls(fixed, self._dimensions)
-
-
-# TODO: Remove this class on DM-38687.
-@deprecated(
-    "Deprecated in favor of configurable dimension packers.  Will be removed after v26.",
-    version="v26",
-    category=FutureWarning,
-)
-class DimensionPackerConstructionVisitor(DimensionConstructionVisitor):
-    """Builder visitor for a single `DimensionPacker`.
-
-    A single `DimensionPackerConstructionVisitor` should be added to a
-    `DimensionConstructionBuilder` for each `DimensionPackerFactory` that
-    should be added to a universe.
-
-    Parameters
-    ----------
-    name : `str`
-        Name used to identify this configuration of the packer in a
-        `DimensionUniverse`.
-    clsName : `str`
-        Fully-qualified name of a `DimensionPacker` subclass.
-    fixed : `~collections.abc.Iterable` [ `str` ]
-        Names of dimensions whose values must be provided to the packer when it
-        is constructed.  This will be expanded lazily into a `DimensionGroup`
-        prior to `DimensionPacker` construction.
-    dimensions : `~collections.abc.Iterable` [ `str` ]
-        Names of dimensions whose values are passed to `DimensionPacker.pack`.
-        This will be expanded lazily into a `DimensionGroup` prior to
-        `DimensionPacker` construction.
-    """
-
-    def __init__(self, name: str, clsName: str, fixed: Iterable[str], dimensions: Iterable[str]):
-        super().__init__(name)
-        self._fixed = set(fixed)
-        self._dimensions = set(dimensions)
-        self._clsName = clsName
-
-    def hasDependenciesIn(self, others: Set[str]) -> bool:
-        # Docstring inherited from DimensionConstructionVisitor.
-        return False
-
-    def visit(self, builder: DimensionConstructionBuilder) -> None:
-        # Docstring inherited from DimensionConstructionVisitor.
-        with warnings.catch_warnings():
-            # Don't warn when deprecated code calls other deprecated code.
-            warnings.simplefilter("ignore", FutureWarning)
-            builder.packers[self.name] = DimensionPackerFactory(
-                clsName=self._clsName,
-                fixed=self._fixed,
-                dimensions=self._dimensions,
-            )

python/lsst/daf/butler/dimensions/_universe.py

@@ -51,8 +51,6 @@
 from ._skypix import SkyPixDimension, SkyPixSystem
 
 if TYPE_CHECKING:  # Imports needed only for type annotations; may be circular.
-    from ._coordinate import DataCoordinate
-    from ._packer import DimensionPacker, DimensionPackerFactory
     from .construction import DimensionConstructionBuilder
 
 
@@ -148,7 +146,6 @@ def __new__(
         self._dimensions = builder.dimensions
         self._elements = builder.elements
         self._topology = builder.topology
-        self._packers = builder.packers
         self.dimensionConfig = builder.config
         commonSkyPix = self._dimensions[builder.commonSkyPixName]
         assert isinstance(commonSkyPix, SkyPixDimension)
@@ -558,31 +555,6 @@ def sorted(self, elements: Iterable[Any], *, reverse: bool = False) -> list[Any]
             result.reverse()
         return result
 
-    # TODO: Remove this method on DM-38687.
-    @deprecated(
-        "Deprecated in favor of configurable dimension packers.  Will be removed after v26.",
-        version="v26",
-        category=FutureWarning,
-    )
-    def makePacker(self, name: str, dataId: DataCoordinate) -> DimensionPacker:
-        """Make a dimension packer.
-
-        Constructs a `DimensionPacker` that can pack data ID dictionaries
-        into unique integers.
-
-        Parameters
-        ----------
-        name : `str`
-            Name of the packer, matching a key in the "packers" section of the
-            dimension configuration.
-        dataId : `DataCoordinate`
-            Fully-expanded data ID that identifies the at least the "fixed"
-            dimensions of the packer (i.e. those that are assumed/given,
-            setting the space over which packed integer IDs are unique).
-            ``dataId.hasRecords()`` must return `True`.
-        """
-        return self._packers[name](self, dataId)
-
     def getEncodeLength(self) -> int:
         """Return encoded size of graph.
 
@@ -674,8 +646,6 @@ def __deepcopy__(self, memo: dict) -> DimensionUniverse:
 
     _elementIndices: dict[str, int]
 
-    _packers: dict[str, DimensionPackerFactory]
-
     _populates: defaultdict[str, NamedValueSet[DimensionElement]]
 
     _version: int

python/lsst/daf/butler/dimensions/construction.py

@@ -37,7 +37,6 @@
 if TYPE_CHECKING:
     from ._config import DimensionConfig
     from ._elements import Dimension, DimensionElement
-    from ._packer import DimensionPackerFactory
 
 
 class DimensionConstructionVisitor(ABC):
@@ -137,7 +136,6 @@ def __init__(
         self.dimensions = NamedValueSet()
         self.elements = NamedValueSet()
         self.topology = {space: NamedValueSet() for space in TopologicalSpace.__members__.values()}
-        self.packers = {}
         self.version = version
         self.namespace = namespace
         self.config = config
@@ -222,8 +220,3 @@ def finish(self) -> None:
     """Dictionary containing all `TopologicalFamily` objects
     (`dict` [ `TopologicalSpace`, `NamedValueSet` [ `TopologicalFamily` ] ] ).
     """
-
-    packers: dict[str, DimensionPackerFactory]
-    """Dictionary containing all `DimensionPackerFactory` objects
-    (`dict` [ `str`, `DimensionPackerFactory` ] ).
-    """