Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DM-38687: Remove deprecated DimensionPacker factory methods. #932

Merged
merged 1 commit into from
Jan 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions doc/changes/DM-38687.removal.md
Original file line number Diff line number Diff line change
@@ -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.
4 changes: 0 additions & 4 deletions doc/lsst.daf.butler/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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: ^"
Expand Down
2 changes: 1 addition & 1 deletion python/lsst/daf/butler/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 *
Expand Down
31 changes: 1 addition & 30 deletions python/lsst/daf/butler/dimensions/_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@

__all__ = ("DimensionConfig",)

import warnings
from collections.abc import Iterator, Mapping, Sequence
from typing import Any

Expand All @@ -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

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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`.

Expand All @@ -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
46 changes: 1 addition & 45 deletions python/lsst/daf/butler/dimensions/_coordinate.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.

Expand Down
125 changes: 0 additions & 125 deletions python/lsst/daf/butler/dimensions/_packer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
)
30 changes: 0 additions & 30 deletions python/lsst/daf/butler/dimensions/_universe.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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.

Expand Down Expand Up @@ -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
Expand Down
7 changes: 0 additions & 7 deletions python/lsst/daf/butler/dimensions/construction.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
if TYPE_CHECKING:
from ._config import DimensionConfig
from ._elements import Dimension, DimensionElement
from ._packer import DimensionPackerFactory


class DimensionConstructionVisitor(ABC):
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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` ] ).
"""
Loading
Loading