Skip to content

Commit

Permalink
Move component handling out of dataset manager.
Browse files Browse the repository at this point in the history
findDataset and getDatasetType are no the only registry methods that
support components; removing support from those would cause a lot of
inconvenience downstream, and the complexity cost in Registry for
keeping it there is quite low. But we want to get it out of the manager
classes to get rid of that unpleasant copy.copy on the storage object,
and to keep component support from accidentally spreading to other
places where we don't want to maintain it.

At the same time, I've changed the exception raised for missing dataset
types in the manager to be MissingDatasetTypeError for consistency with
the query methods, but adding KeyError as a base class of that
exception to maintain backwards compatibility.
  • Loading branch information
TallJimbo committed Oct 1, 2022
1 parent 7027bbb commit 4b57dd7
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 18 deletions.
18 changes: 15 additions & 3 deletions python/lsst/daf/butler/registries/sql.py
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,12 @@ def removeDatasetType(self, name: str) -> None:

def getDatasetType(self, name: str) -> DatasetType:
# Docstring inherited from lsst.daf.butler.registry.Registry
return self._managers.datasets[name].datasetType
parent_name, component = DatasetType.splitDatasetTypeName(name)
storage = self._managers.datasets[parent_name]
if component is None:
return storage.datasetType
else:
return storage.datasetType.makeComponentDatasetType(component)

def supportsIdGenerationMode(self, mode: DatasetIdGenEnum) -> bool:
# Docstring inherited from lsst.daf.butler.registry.Registry
Expand All @@ -435,9 +440,14 @@ def findDataset(
) -> Optional[DatasetRef]:
# Docstring inherited from lsst.daf.butler.registry.Registry
if isinstance(datasetType, DatasetType):
storage = self._managers.datasets[datasetType.name]
parent_name, component = datasetType.nameAndComponent()
storage = self._managers.datasets[parent_name]
else:
storage = self._managers.datasets[datasetType]
parent_name, component = DatasetType.splitDatasetTypeName(datasetType)
storage = self._managers.datasets[parent_name]
datasetType = storage.datasetType
if component is not None:
datasetType.makeComponentDatasetType(component)
dataId = DataCoordinate.standardize(
dataId,
graph=storage.datasetType.dimensions,
Expand All @@ -460,6 +470,8 @@ def findDataset(
continue
result = storage.find(collectionRecord, dataId, timespan=timespan)
if result is not None:
if component is not None:
return result.makeComponentRef(component)
return result

return None
Expand Down
2 changes: 1 addition & 1 deletion python/lsst/daf/butler/registry/_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class DatasetTypeError(RegistryError):
"""Exception raised for problems with dataset types."""


class MissingDatasetTypeError(DatasetTypeError):
class MissingDatasetTypeError(DatasetTypeError, KeyError):
"""Exception raised when a dataset type does not exist."""


Expand Down
14 changes: 11 additions & 3 deletions python/lsst/daf/butler/registry/_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -639,8 +639,13 @@ def getDatasetType(self, name: str) -> DatasetType:
Raises
------
KeyError
Requested named DatasetType could not be found in registry.
MissingDatasetTypeError
Raised if the requested dataset type has not been registered.
Notes
-----
This method handles component dataset types automatically, though most
other registry operations do not.
"""
raise NotImplementedError()

Expand Down Expand Up @@ -712,7 +717,7 @@ def findDataset(
``self.defaults.collections`` is `None`.
LookupError
Raised if one or more data ID keys are missing.
KeyError
MissingDatasetTypeError
Raised if the dataset type does not exist.
MissingCollectionError
Raised if any of ``collections`` does not exist in the registry.
Expand All @@ -728,6 +733,9 @@ def findDataset(
reported consistently, regardless of the reason, and that adding
additional collections that do not contain a match to the search path
never changes the behavior.
This method handles component dataset types automatically, though most
other registry operations do not.
"""
raise NotImplementedError()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
"ByDimensionsDatasetRecordStorageManagerUUID",
)

import copy
import logging
import warnings
from collections import defaultdict
Expand Down Expand Up @@ -254,14 +253,7 @@ def remove(self, name: str) -> None:

def find(self, name: str) -> DatasetRecordStorage | None:
# Docstring inherited from DatasetRecordStorageManager.
compositeName, componentName = DatasetType.splitDatasetTypeName(name)
storage = self._byName.get(compositeName)
if storage is not None and componentName is not None:
componentStorage = copy.copy(storage)
componentStorage.datasetType = storage.datasetType.makeComponentDatasetType(componentName)
return componentStorage
else:
return storage
return self._byName.get(name)

def register(self, datasetType: DatasetType) -> tuple[DatasetRecordStorage, bool]:
# Docstring inherited from DatasetRecordStorageManager.
Expand Down
3 changes: 2 additions & 1 deletion python/lsst/daf/butler/registry/interfaces/_datasets.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import sqlalchemy.sql

from ...core import DataCoordinate, DatasetId, DatasetRef, DatasetType, SimpleQuery, Timespan, ddl
from .._exceptions import MissingDatasetTypeError
from ._versioning import VersionedExtension

if TYPE_CHECKING:
Expand Down Expand Up @@ -596,7 +597,7 @@ def __getitem__(self, name: str) -> DatasetRecordStorage:
"""
result = self.find(name)
if result is None:
raise KeyError(f"Dataset type with name '{name}' not found.")
raise MissingDatasetTypeError(f"Dataset type with name '{name}' not found.")
return result

@abstractmethod
Expand Down
3 changes: 2 additions & 1 deletion python/lsst/daf/butler/registry/tests/_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
DatasetTypeError,
InconsistentDataIdError,
MissingCollectionError,
MissingDatasetTypeError,
OrphanedRecordError,
)
from ..interfaces import ButlerAttributeExistsError, DatasetIdGenEnum
Expand Down Expand Up @@ -433,7 +434,7 @@ def testRemoveDatasetTypeSuccess(self):
registry = self.makeRegistry()
self.loadData(registry, "base.yaml")
registry.removeDatasetType("flat")
with self.assertRaises(KeyError):
with self.assertRaises(MissingDatasetTypeError):
registry.getDatasetType("flat")

def testRemoveDatasetTypeFailure(self):
Expand Down

0 comments on commit 4b57dd7

Please sign in to comment.