From d4f7541212ad39fbde9f78079aac44e871890196 Mon Sep 17 00:00:00 2001 From: Andy Salnikov Date: Mon, 10 Jul 2023 21:49:09 -0700 Subject: [PATCH 1/6] Rename `Butler.registry` attribute to `_registry` This is a trivial change, internally Butler uses `_registry` but it is also exposed as `registry` property. In the next commits `registry` property will return a special shim object, this will allow us to transparently update registry implementation without breaking client-visible interfaces. --- python/lsst/daf/butler/_butler.py | 111 ++++++++++++++++-------------- 1 file changed, 61 insertions(+), 50 deletions(-) diff --git a/python/lsst/daf/butler/_butler.py b/python/lsst/daf/butler/_butler.py index d8b04781d2..8a7c1c8555 100644 --- a/python/lsst/daf/butler/_butler.py +++ b/python/lsst/daf/butler/_butler.py @@ -209,7 +209,7 @@ def __init__( raise TypeError( "Cannot pass 'config', 'searchPaths', or 'writeable' arguments with 'butler' argument." ) - self.registry = butler.registry.copy(defaults) + self._registry = butler._registry.copy(defaults) self._datastore = butler._datastore self.storageClasses = butler.storageClasses self._config: ButlerConfig = butler._config @@ -222,11 +222,11 @@ def __init__( butlerRoot = self._config.configDir if writeable is None: writeable = run is not None - self.registry = Registry.fromConfig( + self._registry = Registry.fromConfig( self._config, butlerRoot=butlerRoot, writeable=writeable, defaults=defaults ) self._datastore = Datastore.fromConfig( - self._config, self.registry.getDatastoreBridgeManager(), butlerRoot=butlerRoot + self._config, self._registry.getDatastoreBridgeManager(), butlerRoot=butlerRoot ) self.storageClasses = StorageClassFactory() self.storageClasses.addFromConfig(self._config) @@ -256,7 +256,7 @@ def __init__( def _retrieve_dataset_type(self, name: str) -> DatasetType | None: """Return DatasetType defined in registry given dataset type name.""" try: - return self.registry.getDatasetType(name) + return self._registry.getDatasetType(name) except MissingDatasetTypeError: return None @@ -517,19 +517,19 @@ def __reduce__(self) -> tuple: self._config, self.collections, self.run, - self.registry.defaults.dataId.byName(), - self.registry.isWriteable(), + self._registry.defaults.dataId.byName(), + self._registry.isWriteable(), ), ) def __str__(self) -> str: return "Butler(collections={}, run={}, datastore='{}', registry='{}')".format( - self.collections, self.run, self._datastore, self.registry + self.collections, self.run, self._datastore, self._registry ) def isWriteable(self) -> bool: """Return `True` if this `Butler` supports write operations.""" - return self.registry.isWriteable() + return self._registry.isWriteable() @contextlib.contextmanager def transaction(self) -> Iterator[None]: @@ -537,7 +537,7 @@ def transaction(self) -> Iterator[None]: Transactions can be nested. """ - with self.registry.transaction(): + with self._registry.transaction(): with self._datastore.transaction(): yield @@ -602,11 +602,11 @@ def _standardizeArgs( if isinstance(datasetRefOrType, DatasetType): externalDatasetType = datasetRefOrType else: - internalDatasetType = self.registry.getDatasetType(datasetRefOrType) + internalDatasetType = self._registry.getDatasetType(datasetRefOrType) # Check that they are self-consistent if externalDatasetType is not None: - internalDatasetType = self.registry.getDatasetType(externalDatasetType.name) + internalDatasetType = self._registry.getDatasetType(externalDatasetType.name) if externalDatasetType != internalDatasetType: # We can allow differences if they are compatible, depending # on whether this is a get or a put. A get requires that @@ -882,7 +882,7 @@ def _rewrite_data_id( ) # Get the actual record and compare with these values. try: - recs = list(self.registry.queryDimensionRecords(dimensionName, dataId=newDataId)) + recs = list(self._registry.queryDimensionRecords(dimensionName, dataId=newDataId)) except DataIdError: raise ValueError( f"Could not find dimension '{dimensionName}'" @@ -911,7 +911,7 @@ def _rewrite_data_id( # Hopefully we get a single record that matches records = set( - self.registry.queryDimensionRecords( + self._registry.queryDimensionRecords( dimensionName, dataId=newDataId, where=where, bind=bind, **kwargs ) ) @@ -927,7 +927,7 @@ def _rewrite_data_id( and "visit_system" in self.dimensions["instrument"].metadata ): instrument_records = list( - self.registry.queryDimensionRecords( + self._registry.queryDimensionRecords( "instrument", dataId=newDataId, **kwargs, @@ -943,7 +943,7 @@ def _rewrite_data_id( # visit_system_membership records. for rec in records: membership = list( - self.registry.queryDimensionRecords( + self._registry.queryDimensionRecords( # Use bind to allow zero results. # This is a fully-specified query. "visit_system_membership", @@ -1055,21 +1055,21 @@ def _findDatasetRef( # dimensions that provide temporal information for a validity-range # lookup. dataId = DataCoordinate.standardize( - dataId, universe=self.dimensions, defaults=self.registry.defaults.dataId, **kwargs + dataId, universe=self.dimensions, defaults=self._registry.defaults.dataId, **kwargs ) if dataId.graph.temporal: - dataId = self.registry.expandDataId(dataId) + dataId = self._registry.expandDataId(dataId) timespan = dataId.timespan else: # Standardize the data ID to just the dimensions of the dataset # type instead of letting registry.findDataset do it, so we get the # result even if no dataset is found. dataId = DataCoordinate.standardize( - dataId, graph=datasetType.dimensions, defaults=self.registry.defaults.dataId, **kwargs + dataId, graph=datasetType.dimensions, defaults=self._registry.defaults.dataId, **kwargs ) # Always lookup the DatasetRef, even if one is given, to ensure it is # present in the current collection. - ref = self.registry.findDataset(datasetType, dataId, collections=collections, timespan=timespan) + ref = self._registry.findDataset(datasetType, dataId, collections=collections, timespan=timespan) if ref is None: if predict: if run is None: @@ -1079,7 +1079,7 @@ def _findDatasetRef( return DatasetRef(datasetType, dataId, run=run) else: if collections is None: - collections = self.registry.defaults.collections + collections = self._registry.defaults.collections raise LookupError( f"Dataset {datasetType.name} with data ID {dataId} " f"could not be found in collections {collections}." @@ -1160,7 +1160,7 @@ def put( # dataset type and DataId, then _importDatasets will do nothing and # just return an original ref. We have to raise in this case, there # is a datastore check below for that. - self.registry._importDatasets([datasetRefOrType], expand=True) + self._registry._importDatasets([datasetRefOrType], expand=True) # Before trying to write to the datastore check that it does not # know this dataset. This is prone to races, of course. if self._datastore.knows(datasetRefOrType): @@ -1183,8 +1183,8 @@ def put( dataId, kwargs = self._rewrite_data_id(dataId, datasetType, **kwargs) # Add Registry Dataset entry. - dataId = self.registry.expandDataId(dataId, graph=datasetType.dimensions, **kwargs) - (ref,) = self.registry.insertDatasets(datasetType, run=run, dataIds=[dataId]) + dataId = self._registry.expandDataId(dataId, graph=datasetType.dimensions, **kwargs) + (ref,) = self._registry.insertDatasets(datasetType, run=run, dataIds=[dataId]) self._datastore.put(obj, ref) return ref @@ -1622,7 +1622,7 @@ def exists( if data_id is not None: warnings.warn("A DataID should not be specified with DatasetRef", stacklevel=2) ref = dataset_ref_or_type - registry_ref = self.registry.getDataset(dataset_ref_or_type.id) + registry_ref = self._registry.getDataset(dataset_ref_or_type.id) if registry_ref is not None: existence |= DatasetExistence.RECORDED @@ -1692,7 +1692,7 @@ def _exists_many( # Registry does not have a bulk API to check for a ref. for ref in refs: - registry_ref = self.registry.getDataset(ref.id) + registry_ref = self._registry.getDataset(ref.id) if registry_ref is not None: # It is possible, albeit unlikely, that the given ref does # not match the one in registry even though the UUID matches. @@ -1769,7 +1769,7 @@ def datasetExists( """ # A resolved ref may be given that is not known to this butler. if isinstance(datasetRefOrType, DatasetRef): - ref = self.registry.getDataset(datasetRefOrType.id) + ref = self._registry.getDataset(datasetRefOrType.id) if ref is None: raise LookupError( f"Resolved DatasetRef with id {datasetRefOrType.id} is not known to registry." @@ -1804,18 +1804,18 @@ def removeRuns(self, names: Iterable[str], unstore: bool = True) -> None: names = list(names) refs: list[DatasetRef] = [] for name in names: - collectionType = self.registry.getCollectionType(name) + collectionType = self._registry.getCollectionType(name) if collectionType is not CollectionType.RUN: raise TypeError(f"The collection type of '{name}' is {collectionType.name}, not RUN.") - refs.extend(self.registry.queryDatasets(..., collections=name, findFirst=True)) + refs.extend(self._registry.queryDatasets(..., collections=name, findFirst=True)) with self._datastore.transaction(): - with self.registry.transaction(): + with self._registry.transaction(): if unstore: self._datastore.trash(refs) else: self._datastore.forget(refs) for name in names: - self.registry.removeCollection(name) + self._registry.removeCollection(name) if unstore: # Point of no return for removing artifacts self._datastore.emptyTrash() @@ -1843,7 +1843,7 @@ def pruneDatasets( if not tags: raise TypeError("No tags provided but disassociate=True.") for tag in tags: - collectionType = self.registry.getCollectionType(tag) + collectionType = self._registry.getCollectionType(tag) if collectionType is not CollectionType.TAGGED: raise TypeError( f"Cannot disassociate from collection '{tag}' " @@ -1864,15 +1864,15 @@ def pruneDatasets( # but shouldn't change them), and hence all operations here are # Registry operations. with self._datastore.transaction(): - with self.registry.transaction(): + with self._registry.transaction(): if unstore: self._datastore.trash(refs) if purge: - self.registry.removeDatasets(refs) + self._registry.removeDatasets(refs) elif disassociate: assert tags, "Guaranteed by earlier logic in this function." for tag in tags: - self.registry.disassociate(tag, refs) + self._registry.disassociate(tag, refs) # We've exited the Registry transaction, and apparently committed. # (if there was an exception, everything rolled back, and it's as if # nothing happened - and we never get here). @@ -2041,7 +2041,7 @@ def ingest( # Import the refs and expand the DataCoordinates since we can't # guarantee that they are expanded and Datastore will need # the records. - imported_refs = self.registry._importDatasets(refs_to_import, expand=True) + imported_refs = self._registry._importDatasets(refs_to_import, expand=True) assert set(imported_refs) == set(refs_to_import) # Replace all the refs in the FileDataset with expanded versions. @@ -2138,7 +2138,7 @@ def export( backend = BackendClass(stream, universe=self.dimensions) try: helper = RepoExportContext( - self.registry, self._datastore, backend=backend, directory=directory, transfer=transfer + self._registry, self._datastore, backend=backend, directory=directory, transfer=transfer ) yield helper except BaseException: @@ -2228,7 +2228,7 @@ def import_( ) def doImport(importStream: TextIO | ResourceHandleProtocol) -> None: - backend = BackendClass(importStream, self.registry) # type: ignore[call-arg] + backend = BackendClass(importStream, self._registry) # type: ignore[call-arg] backend.register() with self.transaction(): backend.load( @@ -2348,11 +2348,11 @@ def transfer_from( # on to find additional inconsistent dataset types # might result in additional unwanted dataset types being # registered. - if self.registry.registerDatasetType(datasetType): + if self._registry.registerDatasetType(datasetType): newly_registered_dataset_types.add(datasetType) else: # If the dataset type is missing, let it fail immediately. - target_dataset_type = self.registry.getDatasetType(datasetType.name) + target_dataset_type = self._registry.getDatasetType(datasetType.name) if target_dataset_type != datasetType: raise ConflictingDefinitionError( "Source butler dataset type differs from definition" @@ -2407,7 +2407,7 @@ def transfer_from( # Assume that if the record is already present that we can # use it without having to check that the record metadata # is consistent. - self.registry.insertDimensionData(element, *records, skip_existing=True) + self._registry.insertDimensionData(element, *records, skip_existing=True) n_imported = 0 for (datasetType, run), refs_to_import in progress.iter_item_chunks( @@ -2419,7 +2419,7 @@ def transfer_from( run_doc = None if registry := getattr(source_butler, "registry", None): run_doc = registry.getCollectionDocumentation(run) - registered = self.registry.registerRun(run, doc=run_doc) + registered = self._registry.registerRun(run, doc=run_doc) handled_collections.add(run) if registered: log.log(VERBOSE, "Creating output run %s", run) @@ -2435,7 +2435,7 @@ def transfer_from( # Assume we are using UUIDs and the source refs will match # those imported. - imported_refs = self.registry._importDatasets(refs_to_import, expand=False) + imported_refs = self._registry._importDatasets(refs_to_import, expand=False) assert set(imported_refs) == set(refs_to_import) n_imported += len(imported_refs) @@ -2493,9 +2493,9 @@ def validateConfiguration( is configured. """ if datasetTypeNames: - datasetTypes = [self.registry.getDatasetType(name) for name in datasetTypeNames] + datasetTypes = [self._registry.getDatasetType(name) for name in datasetTypeNames] else: - datasetTypes = list(self.registry.queryDatasetTypes()) + datasetTypes = list(self._registry.queryDatasetTypes()) # filter out anything from the ignore list if ignore: @@ -2513,7 +2513,7 @@ def validateConfiguration( # Find all the registered instruments (if "instrument" is in the # universe). if "instrument" in self.dimensions: - instruments = {record.name for record in self.registry.queryDimensionRecords("instrument")} + instruments = {record.name for record in self._registry.queryDimensionRecords("instrument")} for datasetType in datasetTypes: if "instrument" in datasetType.dimensions: @@ -2563,7 +2563,7 @@ def validateConfiguration( pass else: try: - self.registry.getDatasetType(key.name) + self._registry.getDatasetType(key.name) except KeyError: if logFailures: log.critical("Key '%s' does not correspond to a DatasetType or StorageClass", key) @@ -2612,7 +2612,7 @@ def collections(self) -> Sequence[str]: by assigning a new `RegistryDefaults` instance to ``self.registry.defaults``. """ - return self.registry.defaults.collections + return self._registry.defaults.collections @property def run(self) -> str | None: @@ -2624,14 +2624,25 @@ def run(self) -> str | None: assigning a new `RegistryDefaults` instance to ``self.registry.defaults``. """ - return self.registry.defaults.run + return self._registry.defaults.run + + @property + def registry(self) -> Registry: + """The object that manages dataset metadata and relationships + (`Registry`). + + Many operations that don't involve reading or writing butler datasets + are accessible only via `Registry` methods. Eventually these methods + will be replaced by equivalent `Butler` methods. + """ + return self._registry @property def dimensions(self) -> DimensionUniverse: # Docstring inherited. - return self.registry.dimensions + return self._registry.dimensions - registry: Registry + _registry: Registry """The object that manages dataset metadata and relationships (`Registry`). Most operations that don't involve reading or writing butler datasets are From c0f152644c182e5721b276bd274601426c51017e Mon Sep 17 00:00:00 2001 From: Andy Salnikov Date: Tue, 11 Jul 2023 11:03:21 -0700 Subject: [PATCH 2/6] Split Registry interface into two classes. `Registry` ABC now contains methods used by regular registry clients, this interface is exposed via `Butler.registry` property. `ButlerRegistry` ABC extends `Registry` with methods that are used by Butler, this includes factory methods and a couple of internal methods. SQL and remote implementations now inherit `ButlerRegistry`, they are not changed otherwise. Also dropped `Registry.datasetIdFactory` attribute, we now have more transparent method for generating dataset IDs and this attribute is not used. --- python/lsst/daf/butler/_butler.py | 10 +- python/lsst/daf/butler/registries/remote.py | 19 +- python/lsst/daf/butler/registries/sql.py | 14 +- python/lsst/daf/butler/registry/__init__.py | 1 + .../daf/butler/registry/_butler_registry.py | 244 ++++++++++++++++++ python/lsst/daf/butler/registry/_registry.py | 213 +-------------- .../daf/butler/registry/tests/_registry.py | 3 +- .../lsst/daf/butler/tests/_datasetsHelper.py | 5 +- python/lsst/daf/butler/transfers/_context.py | 6 +- tests/test_dimensions.py | 5 +- tests/test_obscore.py | 12 +- tests/test_postgresql.py | 8 +- tests/test_quantumBackedButler.py | 4 +- tests/test_query_relations.py | 4 +- tests/test_simpleButler.py | 16 +- tests/test_sqlite.py | 14 +- 16 files changed, 296 insertions(+), 282 deletions(-) create mode 100644 python/lsst/daf/butler/registry/_butler_registry.py diff --git a/python/lsst/daf/butler/_butler.py b/python/lsst/daf/butler/_butler.py index 8a7c1c8555..a50298f751 100644 --- a/python/lsst/daf/butler/_butler.py +++ b/python/lsst/daf/butler/_butler.py @@ -77,6 +77,7 @@ from .core.repoRelocation import BUTLER_ROOT_TAG from .core.utils import transactional from .registry import ( + ButlerRegistry, CollectionType, ConflictingDefinitionError, DataIdError, @@ -222,7 +223,7 @@ def __init__( butlerRoot = self._config.configDir if writeable is None: writeable = run is not None - self._registry = Registry.fromConfig( + self._registry = ButlerRegistry.fromConfig( self._config, butlerRoot=butlerRoot, writeable=writeable, defaults=defaults ) self._datastore = Datastore.fromConfig( @@ -458,7 +459,7 @@ def makeRepo( # Create Registry and populate tables registryConfig = RegistryConfig(config.get("registry")) dimensionConfig = DimensionConfig(dimensionConfig) - Registry.createFromConfig(registryConfig, dimensionConfig=dimensionConfig, butlerRoot=root_uri) + ButlerRegistry.createFromConfig(registryConfig, dimensionConfig=dimensionConfig, butlerRoot=root_uri) log.verbose("Wrote new Butler configuration file to %s", configURI) @@ -2642,8 +2643,9 @@ def dimensions(self) -> DimensionUniverse: # Docstring inherited. return self._registry.dimensions - _registry: Registry - """The object that manages dataset metadata and relationships (`Registry`). + _registry: ButlerRegistry + """The object that manages dataset metadata and relationships + (`ButlerRegistry`). Most operations that don't involve reading or writing butler datasets are accessible only via `Registry` methods. diff --git a/python/lsst/daf/butler/registries/remote.py b/python/lsst/daf/butler/registries/remote.py index 5297da0011..fbb93cc34e 100644 --- a/python/lsst/daf/butler/registries/remote.py +++ b/python/lsst/daf/butler/registries/remote.py @@ -41,7 +41,6 @@ DataId, DatasetAssociation, DatasetId, - DatasetIdFactory, DatasetIdGenEnum, DatasetRef, DatasetType, @@ -66,7 +65,7 @@ QueryDatasetsModel, QueryDimensionRecordsModel, ) -from ..registry import CollectionSummary, CollectionType, Registry, RegistryConfig, RegistryDefaults +from ..registry import ButlerRegistry, CollectionSummary, CollectionType, RegistryConfig, RegistryDefaults if TYPE_CHECKING: from .._butlerConfig import ButlerConfig @@ -74,7 +73,7 @@ from ..registry.interfaces import CollectionRecord, DatastoreRegistryBridgeManager -class RemoteRegistry(Registry): +class RemoteRegistry(ButlerRegistry): """Registry that can talk to a remote Butler server. Parameters @@ -92,8 +91,8 @@ def createFromConfig( config: RegistryConfig | str | None = None, dimensionConfig: DimensionConfig | str | None = None, butlerRoot: ResourcePathExpression | None = None, - ) -> Registry: - """Create registry database and return `Registry` instance. + ) -> ButlerRegistry: + """Create registry database and return `ButlerRegistry` instance. A remote registry can not create a registry database. Calling this method will raise an exception. @@ -107,7 +106,7 @@ def fromConfig( butlerRoot: ResourcePathExpression | None = None, writeable: bool = True, defaults: RegistryDefaults | None = None, - ) -> Registry: + ) -> ButlerRegistry: # Docstring inherited from lsst.daf.butler.registry.Registry config = cls.forceRegistryConfig(config) config.replaceRoot(butlerRoot) @@ -133,10 +132,6 @@ def __init__( self._db = server_uri self._defaults = defaults - # In the future DatasetIdFactory may become configurable and this - # instance will need to be shared with datasets manager. - self.datasetIdFactory = DatasetIdFactory() - # All PUT calls should be short-circuited if not writeable. self._writeable = writeable @@ -167,8 +162,8 @@ def isWriteable(self) -> bool: # Can be used to prevent any PUTs to server return self._writeable - def copy(self, defaults: RegistryDefaults | None = None) -> Registry: - # Docstring inherited from lsst.daf.butler.registry.Registry + def copy(self, defaults: RegistryDefaults | None = None) -> ButlerRegistry: + # Docstring inherited from lsst.daf.butler.registry.ButlerRegistry if defaults is None: # No need to copy, because `RegistryDefaults` is immutable; we # effectively copy on write. diff --git a/python/lsst/daf/butler/registries/sql.py b/python/lsst/daf/butler/registries/sql.py index a216a7ff2a..04c987be76 100644 --- a/python/lsst/daf/butler/registries/sql.py +++ b/python/lsst/daf/butler/registries/sql.py @@ -42,7 +42,6 @@ DatasetAssociation, DatasetColumnTag, DatasetId, - DatasetIdFactory, DatasetIdGenEnum, DatasetRef, DatasetType, @@ -62,6 +61,7 @@ from ..core.utils import transactional from ..registry import ( ArgumentError, + ButlerRegistry, CollectionExpressionError, CollectionSummary, CollectionType, @@ -73,7 +73,6 @@ InconsistentDataIdError, NoDefaultCollectionError, OrphanedRecordError, - Registry, RegistryConfig, RegistryConsistencyError, RegistryDefaults, @@ -97,7 +96,7 @@ _LOG = logging.getLogger(__name__) -class SqlRegistry(Registry): +class SqlRegistry(ButlerRegistry): """Registry implementation based on SQLAlchemy. Parameters @@ -122,7 +121,7 @@ def createFromConfig( config: RegistryConfig | str | None = None, dimensionConfig: DimensionConfig | str | None = None, butlerRoot: ResourcePathExpression | None = None, - ) -> Registry: + ) -> ButlerRegistry: """Create registry database and return `SqlRegistry` instance. This method initializes database contents, database must be empty @@ -169,7 +168,7 @@ def fromConfig( butlerRoot: ResourcePathExpression | None = None, writeable: bool = True, defaults: RegistryDefaults | None = None, - ) -> Registry: + ) -> ButlerRegistry: """Create `Registry` subclass instance from `config`. Registry database must be initialized prior to calling this method. @@ -215,9 +214,6 @@ def __init__(self, database: Database, defaults: RegistryDefaults, managers: Reg # can only be done after most of the rest of Registry has already been # initialized, and must be done before the property getter is used. self.defaults = defaults - # In the future DatasetIdFactory may become configurable and this - # instance will need to be shared with datasets manager. - self.datasetIdFactory = DatasetIdFactory() def __str__(self) -> str: return str(self._db) @@ -229,7 +225,7 @@ def isWriteable(self) -> bool: # Docstring inherited from lsst.daf.butler.registry.Registry return self._db.isWriteable() - def copy(self, defaults: RegistryDefaults | None = None) -> Registry: + def copy(self, defaults: RegistryDefaults | None = None) -> ButlerRegistry: # Docstring inherited from lsst.daf.butler.registry.Registry if defaults is None: # No need to copy, because `RegistryDefaults` is immutable; we diff --git a/python/lsst/daf/butler/registry/__init__.py b/python/lsst/daf/butler/registry/__init__.py index e894782f12..2a346aa3a3 100644 --- a/python/lsst/daf/butler/registry/__init__.py +++ b/python/lsst/daf/butler/registry/__init__.py @@ -20,6 +20,7 @@ # along with this program. If not, see . from . import interfaces, managers, queries, wildcards +from ._butler_registry import ButlerRegistry from ._collection_summary import * from ._collectionType import * from ._config import * diff --git a/python/lsst/daf/butler/registry/_butler_registry.py b/python/lsst/daf/butler/registry/_butler_registry.py new file mode 100644 index 0000000000..7406f7ea61 --- /dev/null +++ b/python/lsst/daf/butler/registry/_butler_registry.py @@ -0,0 +1,244 @@ +# This file is part of daf_butler. +# +# Developed for the LSST Data Management System. +# This product includes software developed by the LSST Project +# (http://www.lsst.org). +# See the COPYRIGHT file at the top-level directory of this distribution +# for details of code ownership. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +from __future__ import annotations + +__all__ = ("ButlerRegistry",) + +from abc import abstractmethod +from typing import TYPE_CHECKING + +from lsst.resources import ResourcePathExpression +from lsst.utils import doImportType + +from ..core import Config, DimensionConfig +from ._config import RegistryConfig +from ._defaults import RegistryDefaults +from ._registry import Registry + +if TYPE_CHECKING: + from .._butlerConfig import ButlerConfig + from .interfaces import CollectionRecord, DatastoreRegistryBridgeManager + + +class ButlerRegistry(Registry): + """Registry interface extended with methods used by Butler implementation. + + Each registry implementation can have its own constructor parameters. + The assumption is that an instance of a specific subclass will be + constructed from configuration using `Registry.fromConfig()`. + The base class will look for a ``cls`` entry and call that specific + `fromConfig()` method. + """ + + defaultConfigFile: str | None = None + """Path to configuration defaults. Accessed within the ``configs`` resource + or relative to a search path. Can be None if no defaults specified. + """ + + @classmethod + def forceRegistryConfig( + cls, config: ButlerConfig | RegistryConfig | Config | str | None + ) -> RegistryConfig: + """Force the supplied config to a `RegistryConfig`. + + Parameters + ---------- + config : `RegistryConfig`, `Config` or `str` or `None` + Registry configuration, if missing then default configuration will + be loaded from registry.yaml. + + Returns + ------- + registry_config : `RegistryConfig` + A registry config. + """ + if not isinstance(config, RegistryConfig): + if isinstance(config, (str, Config)) or config is None: + config = RegistryConfig(config) + else: + raise ValueError(f"Incompatible Registry configuration: {config}") + return config + + @classmethod + def determineTrampoline( + cls, config: ButlerConfig | RegistryConfig | Config | str | None + ) -> tuple[type[ButlerRegistry], RegistryConfig]: + """Return class to use to instantiate real registry. + + Parameters + ---------- + config : `RegistryConfig` or `str`, optional + Registry configuration, if missing then default configuration will + be loaded from registry.yaml. + + Returns + ------- + requested_cls : `type` of `ButlerRegistry` + The real registry class to use. + registry_config : `RegistryConfig` + The `RegistryConfig` to use. + """ + config = cls.forceRegistryConfig(config) + + # Default to the standard registry + registry_cls_name = config.get("cls", "lsst.daf.butler.registries.sql.SqlRegistry") + registry_cls = doImportType(registry_cls_name) + if registry_cls is cls: + raise ValueError("Can not instantiate the abstract base Registry from config") + if not issubclass(registry_cls, ButlerRegistry): + raise TypeError( + f"Registry class obtained from config {registry_cls_name} is not a ButlerRegistry class." + ) + return registry_cls, config + + @classmethod + def createFromConfig( + cls, + config: RegistryConfig | str | None = None, + dimensionConfig: DimensionConfig | str | None = None, + butlerRoot: ResourcePathExpression | None = None, + ) -> ButlerRegistry: + """Create registry database and return `ButlerRegistry` instance. + + This method initializes database contents, database must be empty + prior to calling this method. + + Parameters + ---------- + config : `RegistryConfig` or `str`, optional + Registry configuration, if missing then default configuration will + be loaded from registry.yaml. + dimensionConfig : `DimensionConfig` or `str`, optional + Dimensions configuration, if missing then default configuration + will be loaded from dimensions.yaml. + butlerRoot : convertible to `lsst.resources.ResourcePath`, optional + Path to the repository root this `Registry` will manage. + + Returns + ------- + registry : `ButlerRegistry` + A new `ButlerRegistry` instance. + + Notes + ----- + This class will determine the concrete `ButlerRegistry` subclass to + use from configuration. Each subclass should implement this method + even if it can not create a registry. + """ + registry_cls, registry_config = cls.determineTrampoline(config) + return registry_cls.createFromConfig(registry_config, dimensionConfig, butlerRoot) + + @classmethod + def fromConfig( + cls, + config: ButlerConfig | RegistryConfig | Config | str, + butlerRoot: ResourcePathExpression | None = None, + writeable: bool = True, + defaults: RegistryDefaults | None = None, + ) -> ButlerRegistry: + """Create `ButlerRegistry` subclass instance from ``config``. + + Registry database must be initialized prior to calling this method. + + Parameters + ---------- + config : `ButlerConfig`, `RegistryConfig`, `Config` or `str` + Registry configuration + butlerRoot : convertible to `lsst.resources.ResourcePath`, optional + Path to the repository root this `Registry` will manage. + writeable : `bool`, optional + If `True` (default) create a read-write connection to the database. + defaults : `~lsst.daf.butler.registry.RegistryDefaults`, optional + Default collection search path and/or output `~CollectionType.RUN` + collection. + + Returns + ------- + registry : `ButlerRegistry` (subclass) + A new `ButlerRegistry` subclass instance. + + Notes + ----- + This class will determine the concrete `ButlerRegistry` subclass to + use from configuration. Each subclass should implement this method. + """ + # The base class implementation should trampoline to the correct + # subclass. No implementation should ever use this implementation + # directly. If no class is specified, default to the standard + # registry. + registry_cls, registry_config = cls.determineTrampoline(config) + return registry_cls.fromConfig(config, butlerRoot, writeable, defaults) + + @abstractmethod + def copy(self, defaults: RegistryDefaults | None = None) -> ButlerRegistry: + """Create a new `ButlerRegistry` backed by the same data repository and + connection as this one, but independent defaults. + + Parameters + ---------- + defaults : `~lsst.daf.butler.registry.RegistryDefaults`, optional + Default collections and data ID values for the new registry. If + not provided, ``self.defaults`` will be used (but future changes + to either registry's defaults will not affect the other). + + Returns + ------- + copy : `ButlerRegistry` + A new `ButlerRegistry` instance with its own defaults. + + Notes + ----- + Because the new registry shares a connection with the original, they + also share transaction state (despite the fact that their `transaction` + context manager methods do not reflect this), and must be used with + care. + """ + raise NotImplementedError() + + @abstractmethod + def _get_collection_record(self, name: str) -> CollectionRecord: + """Return the record for this collection. + + Parameters + ---------- + name : `str` + Name of the collection for which the record is to be retrieved. + + Returns + ------- + record : `CollectionRecord` + The record for this collection. + """ + raise NotImplementedError() + + @abstractmethod + def getDatastoreBridgeManager(self) -> DatastoreRegistryBridgeManager: + """Return an object that allows a new `Datastore` instance to + communicate with this `Registry`. + + Returns + ------- + manager : `~.interfaces.DatastoreRegistryBridgeManager` + Object that mediates communication between this `Registry` and its + associated datastores. + """ + raise NotImplementedError() diff --git a/python/lsst/daf/butler/registry/_registry.py b/python/lsst/daf/butler/registry/_registry.py index 8ca9cdadcf..7af253b62e 100644 --- a/python/lsst/daf/butler/registry/_registry.py +++ b/python/lsst/daf/butler/registry/_registry.py @@ -31,21 +31,15 @@ from types import EllipsisType from typing import TYPE_CHECKING, Any -from lsst.resources import ResourcePathExpression -from lsst.utils import doImportType - from ..core import ( - Config, DataCoordinate, DataId, DatasetAssociation, DatasetId, - DatasetIdFactory, DatasetIdGenEnum, DatasetRef, DatasetType, Dimension, - DimensionConfig, DimensionElement, DimensionGraph, DimensionRecord, @@ -56,14 +50,12 @@ ) from ._collection_summary import CollectionSummary from ._collectionType import CollectionType -from ._config import RegistryConfig from ._defaults import RegistryDefaults from .queries import DataCoordinateQueryResults, DatasetQueryResults, DimensionRecordQueryResults from .wildcards import CollectionWildcard if TYPE_CHECKING: - from .._butlerConfig import ButlerConfig - from .interfaces import CollectionRecord, DatastoreRegistryBridgeManager, ObsCoreTableManager + from .interfaces import ObsCoreTableManager _LOG = logging.getLogger(__name__) @@ -74,156 +66,11 @@ class Registry(ABC): """Abstract Registry interface. - Each registry implementation can have its own constructor parameters. - The assumption is that an instance of a specific subclass will be - constructed from configuration using `Registry.fromConfig()`. - The base class will look for a ``cls`` entry and call that specific - `fromConfig()` method. - All subclasses should store `~lsst.daf.butler.registry.RegistryDefaults` in a ``_defaults`` property. No other properties are assumed shared between implementations. """ - defaultConfigFile: str | None = None - """Path to configuration defaults. Accessed within the ``configs`` resource - or relative to a search path. Can be None if no defaults specified. - """ - - @classmethod - def forceRegistryConfig( - cls, config: ButlerConfig | RegistryConfig | Config | str | None - ) -> RegistryConfig: - """Force the supplied config to a `RegistryConfig`. - - Parameters - ---------- - config : `RegistryConfig`, `Config` or `str` or `None` - Registry configuration, if missing then default configuration will - be loaded from registry.yaml. - - Returns - ------- - registry_config : `RegistryConfig` - A registry config. - """ - if not isinstance(config, RegistryConfig): - if isinstance(config, (str, Config)) or config is None: - config = RegistryConfig(config) - else: - raise ValueError(f"Incompatible Registry configuration: {config}") - return config - - @classmethod - def determineTrampoline( - cls, config: ButlerConfig | RegistryConfig | Config | str | None - ) -> tuple[type[Registry], RegistryConfig]: - """Return class to use to instantiate real registry. - - Parameters - ---------- - config : `RegistryConfig` or `str`, optional - Registry configuration, if missing then default configuration will - be loaded from registry.yaml. - - Returns - ------- - requested_cls : `type` of `Registry` - The real registry class to use. - registry_config : `RegistryConfig` - The `RegistryConfig` to use. - """ - config = cls.forceRegistryConfig(config) - - # Default to the standard registry - registry_cls_name = config.get("cls", "lsst.daf.butler.registries.sql.SqlRegistry") - registry_cls = doImportType(registry_cls_name) - if registry_cls is cls: - raise ValueError("Can not instantiate the abstract base Registry from config") - if not issubclass(registry_cls, Registry): - raise TypeError( - f"Registry class obtained from config {registry_cls_name} is not a Registry class." - ) - return registry_cls, config - - @classmethod - def createFromConfig( - cls, - config: RegistryConfig | str | None = None, - dimensionConfig: DimensionConfig | str | None = None, - butlerRoot: ResourcePathExpression | None = None, - ) -> Registry: - """Create registry database and return `Registry` instance. - - This method initializes database contents, database must be empty - prior to calling this method. - - Parameters - ---------- - config : `RegistryConfig` or `str`, optional - Registry configuration, if missing then default configuration will - be loaded from registry.yaml. - dimensionConfig : `DimensionConfig` or `str`, optional - Dimensions configuration, if missing then default configuration - will be loaded from dimensions.yaml. - butlerRoot : convertible to `lsst.resources.ResourcePath`, optional - Path to the repository root this `Registry` will manage. - - Returns - ------- - registry : `Registry` - A new `Registry` instance. - - Notes - ----- - This class will determine the concrete `Registry` subclass to - use from configuration. Each subclass should implement this method - even if it can not create a registry. - """ - registry_cls, registry_config = cls.determineTrampoline(config) - return registry_cls.createFromConfig(registry_config, dimensionConfig, butlerRoot) - - @classmethod - def fromConfig( - cls, - config: ButlerConfig | RegistryConfig | Config | str, - butlerRoot: ResourcePathExpression | None = None, - writeable: bool = True, - defaults: RegistryDefaults | None = None, - ) -> Registry: - """Create `Registry` subclass instance from ``config``. - - Registry database must be initialized prior to calling this method. - - Parameters - ---------- - config : `ButlerConfig`, `RegistryConfig`, `Config` or `str` - Registry configuration - butlerRoot : convertible to `lsst.resources.ResourcePath`, optional - Path to the repository root this `Registry` will manage. - writeable : `bool`, optional - If `True` (default) create a read-write connection to the database. - defaults : `~lsst.daf.butler.registry.RegistryDefaults`, optional - Default collection search path and/or output `~CollectionType.RUN` - collection. - - Returns - ------- - registry : `Registry` (subclass) - A new `Registry` subclass instance. - - Notes - ----- - This class will determine the concrete `Registry` subclass to - use from configuration. Each subclass should implement this method. - """ - # The base class implementation should trampoline to the correct - # subclass. No implementation should ever use this implementation - # directly. If no class is specified, default to the standard - # registry. - registry_cls, registry_config = cls.determineTrampoline(config) - return registry_cls.fromConfig(config, butlerRoot, writeable, defaults) - @abstractmethod def isWriteable(self) -> bool: """Return `True` if this registry allows write operations, and `False` @@ -231,32 +78,6 @@ def isWriteable(self) -> bool: """ raise NotImplementedError() - @abstractmethod - def copy(self, defaults: RegistryDefaults | None = None) -> Registry: - """Create a new `Registry` backed by the same data repository and - connection as this one, but independent defaults. - - Parameters - ---------- - defaults : `~lsst.daf.butler.registry.RegistryDefaults`, optional - Default collections and data ID values for the new registry. If - not provided, ``self.defaults`` will be used (but future changes - to either registry's defaults will not affect the other). - - Returns - ------- - copy : `Registry` - A new `Registry` instance with its own defaults. - - Notes - ----- - Because the new registry shares a connection with the original, they - also share transaction state (despite the fact that their `transaction` - context manager methods do not reflect this), and must be used with - care. - """ - raise NotImplementedError() - @property @abstractmethod def dimensions(self) -> DimensionUniverse: @@ -360,22 +181,6 @@ def getCollectionType(self, name: str) -> CollectionType: """ raise NotImplementedError() - @abstractmethod - def _get_collection_record(self, name: str) -> CollectionRecord: - """Return the record for this collection. - - Parameters - ---------- - name : `str` - Name of the collection for which the record is to be retrieved. - - Returns - ------- - record : `CollectionRecord` - The record for this collection. - """ - raise NotImplementedError() - @abstractmethod def registerRun(self, name: str, doc: str | None = None) -> bool: """Add a new run if one with the given name does not exist. @@ -1017,19 +822,6 @@ def decertify( """ raise NotImplementedError() - @abstractmethod - def getDatastoreBridgeManager(self) -> DatastoreRegistryBridgeManager: - """Return an object that allows a new `Datastore` instance to - communicate with this `Registry`. - - Returns - ------- - manager : `~.interfaces.DatastoreRegistryBridgeManager` - Object that mediates communication between this `Registry` and its - associated datastores. - """ - raise NotImplementedError() - @abstractmethod def getDatasetLocations(self, ref: DatasetRef) -> Iterable[str]: """Retrieve datastore locations for a given dataset. @@ -1678,6 +1470,3 @@ def obsCoreTableManager(self) -> ObsCoreTableManager | None: storageClasses: StorageClassFactory """All storage classes known to the registry (`StorageClassFactory`). """ - - datasetIdFactory: DatasetIdFactory - """Factory for dataset IDs.""" diff --git a/python/lsst/daf/butler/registry/tests/_registry.py b/python/lsst/daf/butler/registry/tests/_registry.py index 435f295690..558ae01f5f 100644 --- a/python/lsst/daf/butler/registry/tests/_registry.py +++ b/python/lsst/daf/butler/registry/tests/_registry.py @@ -49,6 +49,7 @@ DataCoordinate, DataCoordinateSet, DatasetAssociation, + DatasetIdFactory, DatasetIdGenEnum, DatasetRef, DatasetType, @@ -3090,7 +3091,7 @@ def testDatasetIdFactory(self): in its API. """ registry = self.makeRegistry() - factory = registry.datasetIdFactory + factory = DatasetIdFactory() dataset_type = DatasetType( "datasetType", dimensions=["detector", "instrument"], diff --git a/python/lsst/daf/butler/tests/_datasetsHelper.py b/python/lsst/daf/butler/tests/_datasetsHelper.py index ff7e7f0b6a..562cdc2b70 100644 --- a/python/lsst/daf/butler/tests/_datasetsHelper.py +++ b/python/lsst/daf/butler/tests/_datasetsHelper.py @@ -37,7 +37,8 @@ from lsst.daf.butler.formatters.yaml import YamlFormatter if TYPE_CHECKING: - from lsst.daf.butler import Config, DatasetId, Datastore, Dimension, DimensionGraph, Registry + from lsst.daf.butler import Config, DatasetId, Datastore, Dimension, DimensionGraph + from lsst.daf.butler.registry import ButlerRegistry class DatasetTestHelper: @@ -101,7 +102,7 @@ class DatastoreTestHelper: datastoreType: type[Datastore] configFile: str - def setUpDatastoreTests(self, registryClass: type[Registry], configClass: type[Config]) -> None: + def setUpDatastoreTests(self, registryClass: type[ButlerRegistry], configClass: type[Config]) -> None: """Shared setUp code for all Datastore tests.""" self.registry = registryClass() self.config = configClass(self.configFile) diff --git a/python/lsst/daf/butler/transfers/_context.py b/python/lsst/daf/butler/transfers/_context.py index 9b41e83270..aadc0f1445 100644 --- a/python/lsst/daf/butler/transfers/_context.py +++ b/python/lsst/daf/butler/transfers/_context.py @@ -38,7 +38,7 @@ DimensionRecord, FileDataset, ) -from ..registry import CollectionType, Registry +from ..registry import ButlerRegistry, CollectionType from ..registry.interfaces import ChainedCollectionRecord, CollectionRecord from ._interfaces import RepoExportBackend @@ -58,7 +58,7 @@ class RepoExportContext: Parameters ---------- - registry : `Registry` + registry : `ButlerRegistry` Registry to export from. datastore : `Datastore` Datastore to export from. @@ -73,7 +73,7 @@ class RepoExportContext: def __init__( self, - registry: Registry, + registry: ButlerRegistry, datastore: Datastore, backend: RepoExportBackend, *, diff --git a/tests/test_dimensions.py b/tests/test_dimensions.py index 14767065d0..4e17930bcb 100644 --- a/tests/test_dimensions.py +++ b/tests/test_dimensions.py @@ -42,11 +42,10 @@ DimensionUniverse, NamedKeyDict, NamedValueSet, - Registry, TimespanDatabaseRepresentation, YamlRepoImportBackend, ) -from lsst.daf.butler.registry import RegistryConfig +from lsst.daf.butler.registry import ButlerRegistry, RegistryConfig DIMENSION_DATA_FILE = os.path.normpath( os.path.join(os.path.dirname(__file__), "data", "registry", "hsc-rc2-subset.yaml") @@ -65,7 +64,7 @@ def loadDimensionData() -> DataCoordinateSequence: # data and retreive it as a set of DataCoordinate objects. config = RegistryConfig() config["db"] = "sqlite://" - registry = Registry.createFromConfig(config) + registry = ButlerRegistry.createFromConfig(config) with open(DIMENSION_DATA_FILE) as stream: backend = YamlRepoImportBackend(stream, registry) backend.register() diff --git a/tests/test_obscore.py b/tests/test_obscore.py index d50b5b620d..6b1dbddd56 100644 --- a/tests/test_obscore.py +++ b/tests/test_obscore.py @@ -33,13 +33,12 @@ CollectionType, Config, DataCoordinate, - DatasetIdGenEnum, DatasetRef, DatasetType, StorageClassFactory, ) from lsst.daf.butler.registries.sql import SqlRegistry -from lsst.daf.butler.registry import Registry, RegistryConfig +from lsst.daf.butler.registry import ButlerRegistry, Registry, RegistryConfig from lsst.daf.butler.registry.obscore import ( DatasetTypeConfig, ObsCoreConfig, @@ -65,10 +64,10 @@ class ObsCoreTests(TestCaseMixin): def make_registry( self, collections: list[str] | None = None, collection_type: str | None = None - ) -> Registry: + ) -> ButlerRegistry: """Create new empty Registry.""" config = self.make_registry_config(collections, collection_type) - registry = Registry.createFromConfig(config, butlerRoot=self.root) + registry = ButlerRegistry.createFromConfig(config, butlerRoot=self.root) self.initialize_registry(registry) return registry @@ -225,10 +224,7 @@ def _insert_dataset( coordinate = DataCoordinate.standardize(data_id, universe=registry.dimensions) if do_import: ds_type = self.dataset_types[dataset_type] - dataset_id = registry.datasetIdFactory.makeDatasetId( - run, ds_type, coordinate, DatasetIdGenEnum.UNIQUE - ) - ref = DatasetRef(ds_type, coordinate, id=dataset_id, run=run) + ref = DatasetRef(ds_type, coordinate, run=run) [ref] = registry._importDatasets([ref]) else: [ref] = registry.insertDatasets(dataset_type, [data_id], run=run) diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 78dc6c2308..5f139db579 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -40,7 +40,7 @@ import sqlalchemy from lsst.daf.butler import Timespan, ddl -from lsst.daf.butler.registry import Registry +from lsst.daf.butler.registry import ButlerRegistry from lsst.daf.butler.registry.databases.postgresql import PostgresqlDatabase, _RangeTimespanType from lsst.daf.butler.registry.tests import DatabaseTests, RegistryTests from lsst.daf.butler.tests.utils import makeTestTempDir, removeTestTempDir @@ -236,7 +236,7 @@ def tearDownClass(cls): def getDataDir(cls) -> str: return os.path.normpath(os.path.join(os.path.dirname(__file__), "data", "registry")) - def makeRegistry(self, share_repo_with: Registry | None = None) -> Registry: + def makeRegistry(self, share_repo_with: ButlerRegistry | None = None) -> ButlerRegistry: if share_repo_with is None: namespace = f"namespace_{secrets.token_hex(8).lower()}" else: @@ -245,9 +245,9 @@ def makeRegistry(self, share_repo_with: Registry | None = None) -> Registry: config["db"] = self.server.url() config["namespace"] = namespace if share_repo_with is None: - return Registry.createFromConfig(config) + return ButlerRegistry.createFromConfig(config) else: - return Registry.fromConfig(config) + return ButlerRegistry.fromConfig(config) class PostgresqlRegistryNameKeyCollMgrUUIDTestCase(PostgresqlRegistryTests, unittest.TestCase): diff --git a/tests/test_quantumBackedButler.py b/tests/test_quantumBackedButler.py index 18c6c898e7..c027990f21 100644 --- a/tests/test_quantumBackedButler.py +++ b/tests/test_quantumBackedButler.py @@ -34,10 +34,10 @@ Quantum, QuantumBackedButler, QuantumProvenanceData, - Registry, RegistryConfig, StorageClass, ) +from lsst.daf.butler.registry import ButlerRegistry from lsst.daf.butler.tests.utils import makeTestTempDir, removeTestTempDir from lsst.resources import ResourcePath @@ -55,7 +55,7 @@ def setUp(self) -> None: # Make a butler and import dimension definitions. registryConfig = RegistryConfig(self.config.get("registry")) - Registry.createFromConfig(registryConfig, butlerRoot=self.root) + ButlerRegistry.createFromConfig(registryConfig, butlerRoot=self.root) self.butler = Butler(self.config, writeable=True, run="RUN") self.butler.import_(filename=os.path.join(TESTDIR, "data", "registry", "base.yaml")) diff --git a/tests/test_query_relations.py b/tests/test_query_relations.py index ffead5de84..120d777b87 100644 --- a/tests/test_query_relations.py +++ b/tests/test_query_relations.py @@ -25,7 +25,7 @@ import re import unittest -from lsst.daf.butler.registry import MissingSpatialOverlapError, Registry, RegistryConfig, queries +from lsst.daf.butler.registry import ButlerRegistry, MissingSpatialOverlapError, RegistryConfig, queries from lsst.daf.butler.transfers import YamlRepoImportBackend TESTDIR = os.path.abspath(os.path.dirname(__file__)) @@ -52,7 +52,7 @@ class TestQueryRelationsTests(unittest.TestCase): def setUpClass(cls) -> None: config = RegistryConfig() config["db"] = "sqlite://" - cls.registry = Registry.createFromConfig(config) + cls.registry = ButlerRegistry.createFromConfig(config) # We need just enough test data to have valid dimension records for # all of the dimensions we're concerned with, and we want to pick # values for each dimension that correspond to a spatiotemporal diff --git a/tests/test_simpleButler.py b/tests/test_simpleButler.py index 2a25317f05..8178438da0 100644 --- a/tests/test_simpleButler.py +++ b/tests/test_simpleButler.py @@ -34,17 +34,8 @@ np = None import astropy.time -from lsst.daf.butler import ( - Butler, - ButlerConfig, - CollectionType, - DatasetId, - DatasetRef, - DatasetType, - Registry, - Timespan, -) -from lsst.daf.butler.registry import RegistryConfig, RegistryDefaults +from lsst.daf.butler import Butler, ButlerConfig, CollectionType, DatasetId, DatasetRef, DatasetType, Timespan +from lsst.daf.butler.registry import ButlerRegistry, RegistryConfig, RegistryDefaults from lsst.daf.butler.tests import DatastoreMock from lsst.daf.butler.tests.utils import makeTestTempDir, removeTestTempDir @@ -80,7 +71,7 @@ def makeButler(self, **kwargs: Any) -> Butler: # have to make a registry first registryConfig = RegistryConfig(config.get("registry")) - Registry.createFromConfig(registryConfig) + ButlerRegistry.createFromConfig(registryConfig) butler = Butler(config, **kwargs) DatastoreMock.apply(butler) @@ -286,7 +277,6 @@ def testButlerGet(self): # Create a numpy integer to check that works fine detector_np = np.int64(2) if np else 2 - print(type(detector_np)) # Try to get it using different variations of dataId + keyword # arguments diff --git a/tests/test_sqlite.py b/tests/test_sqlite.py index bf76af233e..48467c09c2 100644 --- a/tests/test_sqlite.py +++ b/tests/test_sqlite.py @@ -28,7 +28,7 @@ import sqlalchemy from lsst.daf.butler import ddl -from lsst.daf.butler.registry import Registry +from lsst.daf.butler.registry import ButlerRegistry from lsst.daf.butler.registry.databases.sqlite import SqliteDatabase from lsst.daf.butler.registry.tests import DatabaseTests, RegistryTests from lsst.daf.butler.tests.utils import makeTestTempDir, removeTestTempDir @@ -196,7 +196,7 @@ def tearDown(self): def getDataDir(cls) -> str: return os.path.normpath(os.path.join(os.path.dirname(__file__), "data", "registry")) - def makeRegistry(self, share_repo_with: Registry | None = None) -> Registry: + def makeRegistry(self, share_repo_with: ButlerRegistry | None = None) -> ButlerRegistry: if share_repo_with is None: _, filename = tempfile.mkstemp(dir=self.root, suffix=".sqlite3") else: @@ -204,9 +204,9 @@ def makeRegistry(self, share_repo_with: Registry | None = None) -> Registry: config = self.makeRegistryConfig() config["db"] = f"sqlite:///{filename}" if share_repo_with is None: - return Registry.createFromConfig(config, butlerRoot=self.root) + return ButlerRegistry.createFromConfig(config, butlerRoot=self.root) else: - return Registry.fromConfig(config, butlerRoot=self.root) + return ButlerRegistry.fromConfig(config, butlerRoot=self.root) class SqliteFileRegistryNameKeyCollMgrUUIDTestCase(SqliteFileRegistryTests, unittest.TestCase): @@ -242,12 +242,12 @@ class SqliteMemoryRegistryTests(RegistryTests): def getDataDir(cls) -> str: return os.path.normpath(os.path.join(os.path.dirname(__file__), "data", "registry")) - def makeRegistry(self, share_repo_with: Registry | None = None) -> Registry | None: + def makeRegistry(self, share_repo_with: ButlerRegistry | None = None) -> ButlerRegistry | None: if share_repo_with is not None: return None config = self.makeRegistryConfig() config["db"] = "sqlite://" - return Registry.createFromConfig(config) + return ButlerRegistry.createFromConfig(config) def testMissingAttributes(self): """Test for instantiating a registry against outdated schema which @@ -258,7 +258,7 @@ def testMissingAttributes(self): config = self.makeRegistryConfig() config["db"] = "sqlite://" with self.assertRaises(LookupError): - Registry.fromConfig(config) + ButlerRegistry.fromConfig(config) class SqliteMemoryRegistryNameKeyCollMgrUUIDTestCase(unittest.TestCase, SqliteMemoryRegistryTests): From 1e82440f5cd0d7dd26e5c0e2941f50a3c1bf6736 Mon Sep 17 00:00:00 2001 From: Andy Salnikov Date: Tue, 11 Jul 2023 15:25:50 -0700 Subject: [PATCH 3/6] Add `RegistryShim` class. This new class implements `Registry` interface and forwards all methods to other objects. Everything is forwarded to `Butler._registry` now, but this will change as we start refactoring ButlerRegistry implementation. --- python/lsst/daf/butler/_butler.py | 5 +- python/lsst/daf/butler/_registry_shim.py | 392 +++++++++++++++++++++++ tests/test_butler.py | 4 +- 3 files changed, 398 insertions(+), 3 deletions(-) create mode 100644 python/lsst/daf/butler/_registry_shim.py diff --git a/python/lsst/daf/butler/_butler.py b/python/lsst/daf/butler/_butler.py index a50298f751..7bc77e2dfd 100644 --- a/python/lsst/daf/butler/_butler.py +++ b/python/lsst/daf/butler/_butler.py @@ -51,6 +51,7 @@ from ._dataset_existence import DatasetExistence from ._deferredDatasetHandle import DeferredDatasetHandle from ._limited_butler import LimitedButler +from ._registry_shim import RegistryShim from .core import ( Config, ConfigSubset, @@ -246,6 +247,8 @@ def __init__( if "run" in self._config or "collection" in self._config: raise ValueError("Passing a run or collection via configuration is no longer supported.") + self._registry_shim = RegistryShim(self) + GENERATION: ClassVar[int] = 3 """This is a Generation 3 Butler. @@ -2636,7 +2639,7 @@ def registry(self) -> Registry: are accessible only via `Registry` methods. Eventually these methods will be replaced by equivalent `Butler` methods. """ - return self._registry + return self._registry_shim @property def dimensions(self) -> DimensionUniverse: diff --git a/python/lsst/daf/butler/_registry_shim.py b/python/lsst/daf/butler/_registry_shim.py new file mode 100644 index 0000000000..3cc12f3944 --- /dev/null +++ b/python/lsst/daf/butler/_registry_shim.py @@ -0,0 +1,392 @@ +# This file is part of daf_butler. +# +# Developed for the LSST Data Management System. +# This product includes software developed by the LSST Project +# (http://www.lsst.org). +# See the COPYRIGHT file at the top-level directory of this distribution +# for details of code ownership. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +from __future__ import annotations + +__all__ = ("Registry",) + +import contextlib +from collections.abc import Iterable, Iterator, Mapping, Sequence +from typing import TYPE_CHECKING, Any + +from .core import ( + DataCoordinate, + DataId, + DatasetAssociation, + DatasetId, + DatasetIdGenEnum, + DatasetRef, + DatasetType, + Dimension, + DimensionElement, + DimensionGraph, + DimensionRecord, + DimensionUniverse, + NameLookupMapping, + Timespan, +) +from .registry import Registry +from .registry._collection_summary import CollectionSummary +from .registry._collectionType import CollectionType +from .registry._defaults import RegistryDefaults +from .registry.queries import DataCoordinateQueryResults, DatasetQueryResults, DimensionRecordQueryResults + +if TYPE_CHECKING: + from ._butler import Butler + from .registry._registry import CollectionArgType + from .registry.interfaces import ObsCoreTableManager + + +class RegistryShim(Registry): + """Implementation of `Registry` interface exposed to clients by `Butler`. + + Parameters + ---------- + butler : `Butler` + Data butler instance. + + Notes + ----- + This shim implementation of `Registry` forwards all methods to an actual + Registry instance which is internal to Butler or to Butler methods. Its + purpose is to provide a stable interface to many client-visible operations + while we perform re-structuring of Registry and Butler implementations. + """ + + def __init__(self, butler: Butler): + self._butler = butler + self._registry = butler._registry + + def isWriteable(self) -> bool: + # Docstring inherited from a base class. + return self._registry.isWriteable() + + @property + def dimensions(self) -> DimensionUniverse: + # Docstring inherited from a base class. + return self._registry.dimensions + + @property + def defaults(self) -> RegistryDefaults: + # Docstring inherited from a base class. + return self._registry.defaults + + @defaults.setter + def defaults(self, value: RegistryDefaults) -> None: + # Docstring inherited from a base class. + self._registry.defaults = value + + def refresh(self) -> None: + # Docstring inherited from a base class. + self._registry.refresh() + + @contextlib.contextmanager + def transaction(self, *, savepoint: bool = False) -> Iterator[None]: + # Docstring inherited from a base class. + with self._registry.transaction(savepoint=savepoint): + yield + + def resetConnectionPool(self) -> None: + # Docstring inherited from a base class. + self._registry.resetConnectionPool() + + def registerCollection( + self, name: str, type: CollectionType = CollectionType.TAGGED, doc: str | None = None + ) -> bool: + # Docstring inherited from a base class. + return self._registry.registerCollection(name, type, doc) + + def getCollectionType(self, name: str) -> CollectionType: + # Docstring inherited from a base class. + return self._registry.getCollectionType(name) + + def registerRun(self, name: str, doc: str | None = None) -> bool: + # Docstring inherited from a base class. + return self._registry.registerRun(name, doc) + + def removeCollection(self, name: str) -> None: + # Docstring inherited from a base class. + self._registry.removeCollection(name) + + def getCollectionChain(self, parent: str) -> Sequence[str]: + # Docstring inherited from a base class. + return self._registry.getCollectionChain(parent) + + def setCollectionChain(self, parent: str, children: Any, *, flatten: bool = False) -> None: + # Docstring inherited from a base class. + self._registry.setCollectionChain(parent, children, flatten=flatten) + + def getCollectionParentChains(self, collection: str) -> set[str]: + # Docstring inherited from a base class. + return self._registry.getCollectionParentChains(collection) + + def getCollectionDocumentation(self, collection: str) -> str | None: + # Docstring inherited from a base class. + return self._registry.getCollectionDocumentation(collection) + + def setCollectionDocumentation(self, collection: str, doc: str | None) -> None: + # Docstring inherited from a base class. + self._registry.setCollectionDocumentation(collection, doc) + + def getCollectionSummary(self, collection: str) -> CollectionSummary: + # Docstring inherited from a base class. + return self._registry.getCollectionSummary(collection) + + def registerDatasetType(self, datasetType: DatasetType) -> bool: + # Docstring inherited from a base class. + return self._registry.registerDatasetType(datasetType) + + def removeDatasetType(self, name: str | tuple[str, ...]) -> None: + # Docstring inherited from a base class. + self._registry.removeDatasetType(name) + + def getDatasetType(self, name: str) -> DatasetType: + # Docstring inherited from a base class. + return self._registry.getDatasetType(name) + + def supportsIdGenerationMode(self, mode: DatasetIdGenEnum) -> bool: + # Docstring inherited from a base class. + return self._registry.supportsIdGenerationMode(mode) + + def findDataset( + self, + datasetType: DatasetType | str, + dataId: DataId | None = None, + *, + collections: CollectionArgType | None = None, + timespan: Timespan | None = None, + **kwargs: Any, + ) -> DatasetRef | None: + # Docstring inherited from a base class. + return self._registry.findDataset( + datasetType, dataId, collections=collections, timespan=timespan, **kwargs + ) + + def insertDatasets( + self, + datasetType: DatasetType | str, + dataIds: Iterable[DataId], + run: str | None = None, + expand: bool = True, + idGenerationMode: DatasetIdGenEnum = DatasetIdGenEnum.UNIQUE, + ) -> list[DatasetRef]: + # Docstring inherited from a base class. + return self._registry.insertDatasets(datasetType, dataIds, run, expand, idGenerationMode) + + def _importDatasets(self, datasets: Iterable[DatasetRef], expand: bool = True) -> list[DatasetRef]: + # Docstring inherited from a base class. + return self._registry._importDatasets(datasets, expand) + + def getDataset(self, id: DatasetId) -> DatasetRef | None: + # Docstring inherited from a base class. + return self._registry.getDataset(id) + + def removeDatasets(self, refs: Iterable[DatasetRef]) -> None: + # Docstring inherited from a base class. + self._registry.removeDatasets(refs) + + def associate(self, collection: str, refs: Iterable[DatasetRef]) -> None: + # Docstring inherited from a base class. + self._registry.associate(collection, refs) + + def disassociate(self, collection: str, refs: Iterable[DatasetRef]) -> None: + # Docstring inherited from a base class. + self._registry.disassociate(collection, refs) + + def certify(self, collection: str, refs: Iterable[DatasetRef], timespan: Timespan) -> None: + # Docstring inherited from a base class. + self._registry.certify(collection, refs, timespan) + + def decertify( + self, + collection: str, + datasetType: str | DatasetType, + timespan: Timespan, + *, + dataIds: Iterable[DataId] | None = None, + ) -> None: + # Docstring inherited from a base class. + self._registry.decertify(collection, datasetType, timespan, dataIds=dataIds) + + def getDatasetLocations(self, ref: DatasetRef) -> Iterable[str]: + # Docstring inherited from a base class. + return self._registry.getDatasetLocations(ref) + + def expandDataId( + self, + dataId: DataId | None = None, + *, + graph: DimensionGraph | None = None, + records: NameLookupMapping[DimensionElement, DimensionRecord | None] | None = None, + withDefaults: bool = True, + **kwargs: Any, + ) -> DataCoordinate: + # Docstring inherited from a base class. + return self._registry.expandDataId( + dataId, graph=graph, records=records, withDefaults=withDefaults, **kwargs + ) + + def insertDimensionData( + self, + element: DimensionElement | str, + *data: Mapping[str, Any] | DimensionRecord, + conform: bool = True, + replace: bool = False, + skip_existing: bool = False, + ) -> None: + # Docstring inherited from a base class. + self._registry.insertDimensionData( + element, *data, conform=conform, replace=replace, skip_existing=skip_existing + ) + + def syncDimensionData( + self, + element: DimensionElement | str, + row: Mapping[str, Any] | DimensionRecord, + conform: bool = True, + update: bool = False, + ) -> bool | dict[str, Any]: + # Docstring inherited from a base class. + return self._registry.syncDimensionData(element, row, conform, update) + + def queryDatasetTypes( + self, + expression: Any = ..., + *, + components: bool | None = None, + missing: list[str] | None = None, + ) -> Iterable[DatasetType]: + # Docstring inherited from a base class. + return self._registry.queryDatasetTypes(expression, components=components, missing=missing) + + def queryCollections( + self, + expression: Any = ..., + datasetType: DatasetType | None = None, + collectionTypes: Iterable[CollectionType] | CollectionType = CollectionType.all(), + flattenChains: bool = False, + includeChains: bool | None = None, + ) -> Sequence[str]: + # Docstring inherited from a base class. + return self._registry.queryCollections( + expression, datasetType, collectionTypes, flattenChains, includeChains + ) + + def queryDatasets( + self, + datasetType: Any, + *, + collections: CollectionArgType | None = None, + dimensions: Iterable[Dimension | str] | None = None, + dataId: DataId | None = None, + where: str = "", + findFirst: bool = False, + components: bool | None = None, + bind: Mapping[str, Any] | None = None, + check: bool = True, + **kwargs: Any, + ) -> DatasetQueryResults: + # Docstring inherited from a base class. + return self._registry.queryDatasets( + datasetType, + collections=collections, + dimensions=dimensions, + dataId=dataId, + where=where, + findFirst=findFirst, + components=components, + bind=bind, + check=check, + **kwargs, + ) + + def queryDataIds( + self, + dimensions: Iterable[Dimension | str] | Dimension | str, + *, + dataId: DataId | None = None, + datasets: Any = None, + collections: CollectionArgType | None = None, + where: str = "", + components: bool | None = None, + bind: Mapping[str, Any] | None = None, + check: bool = True, + **kwargs: Any, + ) -> DataCoordinateQueryResults: + # Docstring inherited from a base class. + return self._registry.queryDataIds( + dimensions, + dataId=dataId, + datasets=datasets, + collections=collections, + where=where, + components=components, + bind=bind, + check=check, + **kwargs, + ) + + def queryDimensionRecords( + self, + element: DimensionElement | str, + *, + dataId: DataId | None = None, + datasets: Any = None, + collections: CollectionArgType | None = None, + where: str = "", + components: bool | None = None, + bind: Mapping[str, Any] | None = None, + check: bool = True, + **kwargs: Any, + ) -> DimensionRecordQueryResults: + # Docstring inherited from a base class. + return self._registry.queryDimensionRecords( + element, + dataId=dataId, + datasets=datasets, + collections=collections, + where=where, + components=components, + bind=bind, + check=check, + **kwargs, + ) + + def queryDatasetAssociations( + self, + datasetType: str | DatasetType, + collections: CollectionArgType | None = ..., + *, + collectionTypes: Iterable[CollectionType] = CollectionType.all(), + flattenChains: bool = False, + ) -> Iterator[DatasetAssociation]: + # Docstring inherited from a base class. + return self._registry.queryDatasetAssociations( + datasetType, + collections, + collectionTypes=collectionTypes, + flattenChains=flattenChains, + ) + + @property + def obsCoreTableManager(self) -> ObsCoreTableManager | None: + # Docstring inherited from a base class. + return self._registry.obsCoreTableManager diff --git a/tests/test_butler.py b/tests/test_butler.py index 605f986227..307cad7573 100644 --- a/tests/test_butler.py +++ b/tests/test_butler.py @@ -1679,8 +1679,8 @@ def testPytypeCoercion(self) -> None: # Now need to hack the registry dataset type definition. # There is no API for this. - assert isinstance(butler.registry, SqlRegistry) - manager = butler.registry._managers.datasets + assert isinstance(butler._registry, SqlRegistry) + manager = butler._registry._managers.datasets assert hasattr(manager, "_db") and hasattr(manager, "_static") manager._db.update( manager._static.dataset_type, From 020fe5b5e14400d62f62bbe4f1bc05ed36f03220 Mon Sep 17 00:00:00 2001 From: Andy Salnikov Date: Tue, 11 Jul 2023 19:38:37 -0700 Subject: [PATCH 4/6] Move registry factory methods to a separate class. Trying to further separate concerns into independent classes. Re-introduced `Registry.createFromConfig` to avoid updating tests in many other packages. --- python/lsst/daf/butler/_butler.py | 9 +- python/lsst/daf/butler/registry/__init__.py | 1 + .../daf/butler/registry/_butler_registry.py | 41 +----- python/lsst/daf/butler/registry/_registry.py | 43 ++++++ .../daf/butler/registry/_registry_factory.py | 128 ++++++++++++++++++ tests/test_dimensions.py | 4 +- tests/test_obscore.py | 4 +- tests/test_postgresql.py | 6 +- tests/test_quantumBackedButler.py | 4 +- tests/test_query_relations.py | 4 +- tests/test_simpleButler.py | 4 +- tests/test_sqlite.py | 10 +- 12 files changed, 200 insertions(+), 58 deletions(-) create mode 100644 python/lsst/daf/butler/registry/_registry_factory.py diff --git a/python/lsst/daf/butler/_butler.py b/python/lsst/daf/butler/_butler.py index 7bc77e2dfd..34b7ab5109 100644 --- a/python/lsst/daf/butler/_butler.py +++ b/python/lsst/daf/butler/_butler.py @@ -87,6 +87,7 @@ Registry, RegistryConfig, RegistryDefaults, + RegistryFactory, ) from .transfers import RepoExportContext @@ -224,8 +225,8 @@ def __init__( butlerRoot = self._config.configDir if writeable is None: writeable = run is not None - self._registry = ButlerRegistry.fromConfig( - self._config, butlerRoot=butlerRoot, writeable=writeable, defaults=defaults + self._registry = RegistryFactory(self._config).from_config( + butlerRoot=butlerRoot, writeable=writeable, defaults=defaults ) self._datastore = Datastore.fromConfig( self._config, self._registry.getDatastoreBridgeManager(), butlerRoot=butlerRoot @@ -462,7 +463,9 @@ def makeRepo( # Create Registry and populate tables registryConfig = RegistryConfig(config.get("registry")) dimensionConfig = DimensionConfig(dimensionConfig) - ButlerRegistry.createFromConfig(registryConfig, dimensionConfig=dimensionConfig, butlerRoot=root_uri) + RegistryFactory(registryConfig).create_from_config( + dimensionConfig=dimensionConfig, butlerRoot=root_uri + ) log.verbose("Wrote new Butler configuration file to %s", configURI) diff --git a/python/lsst/daf/butler/registry/__init__.py b/python/lsst/daf/butler/registry/__init__.py index 2a346aa3a3..6151e93c24 100644 --- a/python/lsst/daf/butler/registry/__init__.py +++ b/python/lsst/daf/butler/registry/__init__.py @@ -28,6 +28,7 @@ from ._defaults import * from ._exceptions import * from ._registry import * +from ._registry_factory import * from .wildcards import CollectionSearch # Some modules intentionally not imported, either because they are purely diff --git a/python/lsst/daf/butler/registry/_butler_registry.py b/python/lsst/daf/butler/registry/_butler_registry.py index 7406f7ea61..8a6d3bb633 100644 --- a/python/lsst/daf/butler/registry/_butler_registry.py +++ b/python/lsst/daf/butler/registry/_butler_registry.py @@ -27,7 +27,6 @@ from typing import TYPE_CHECKING from lsst.resources import ResourcePathExpression -from lsst.utils import doImportType from ..core import Config, DimensionConfig from ._config import RegistryConfig @@ -79,38 +78,7 @@ def forceRegistryConfig( return config @classmethod - def determineTrampoline( - cls, config: ButlerConfig | RegistryConfig | Config | str | None - ) -> tuple[type[ButlerRegistry], RegistryConfig]: - """Return class to use to instantiate real registry. - - Parameters - ---------- - config : `RegistryConfig` or `str`, optional - Registry configuration, if missing then default configuration will - be loaded from registry.yaml. - - Returns - ------- - requested_cls : `type` of `ButlerRegistry` - The real registry class to use. - registry_config : `RegistryConfig` - The `RegistryConfig` to use. - """ - config = cls.forceRegistryConfig(config) - - # Default to the standard registry - registry_cls_name = config.get("cls", "lsst.daf.butler.registries.sql.SqlRegistry") - registry_cls = doImportType(registry_cls_name) - if registry_cls is cls: - raise ValueError("Can not instantiate the abstract base Registry from config") - if not issubclass(registry_cls, ButlerRegistry): - raise TypeError( - f"Registry class obtained from config {registry_cls_name} is not a ButlerRegistry class." - ) - return registry_cls, config - - @classmethod + @abstractmethod def createFromConfig( cls, config: RegistryConfig | str | None = None, @@ -144,10 +112,10 @@ def createFromConfig( use from configuration. Each subclass should implement this method even if it can not create a registry. """ - registry_cls, registry_config = cls.determineTrampoline(config) - return registry_cls.createFromConfig(registry_config, dimensionConfig, butlerRoot) + raise NotImplementedError() @classmethod + @abstractmethod def fromConfig( cls, config: ButlerConfig | RegistryConfig | Config | str, @@ -185,8 +153,7 @@ def fromConfig( # subclass. No implementation should ever use this implementation # directly. If no class is specified, default to the standard # registry. - registry_cls, registry_config = cls.determineTrampoline(config) - return registry_cls.fromConfig(config, butlerRoot, writeable, defaults) + raise NotImplementedError() @abstractmethod def copy(self, defaults: RegistryDefaults | None = None) -> ButlerRegistry: diff --git a/python/lsst/daf/butler/registry/_registry.py b/python/lsst/daf/butler/registry/_registry.py index 7af253b62e..99c2080fdb 100644 --- a/python/lsst/daf/butler/registry/_registry.py +++ b/python/lsst/daf/butler/registry/_registry.py @@ -31,6 +31,8 @@ from types import EllipsisType from typing import TYPE_CHECKING, Any +from lsst.resources import ResourcePathExpression + from ..core import ( DataCoordinate, DataId, @@ -40,6 +42,7 @@ DatasetRef, DatasetType, Dimension, + DimensionConfig, DimensionElement, DimensionGraph, DimensionRecord, @@ -50,6 +53,7 @@ ) from ._collection_summary import CollectionSummary from ._collectionType import CollectionType +from ._config import RegistryConfig from ._defaults import RegistryDefaults from .queries import DataCoordinateQueryResults, DatasetQueryResults, DimensionRecordQueryResults from .wildcards import CollectionWildcard @@ -71,6 +75,45 @@ class Registry(ABC): implementations. """ + @classmethod + def createFromConfig( + cls, + config: RegistryConfig | str | None = None, + dimensionConfig: DimensionConfig | str | None = None, + butlerRoot: ResourcePathExpression | None = None, + ) -> Registry: + """Create registry database and return `Registry` instance. + + This method initializes database contents, database must be empty + prior to calling this method. + + Parameters + ---------- + config : `RegistryConfig` or `str`, optional + Registry configuration, if missing then default configuration will + be loaded from registry.yaml. + dimensionConfig : `DimensionConfig` or `str`, optional + Dimensions configuration, if missing then default configuration + will be loaded from dimensions.yaml. + butlerRoot : convertible to `lsst.resources.ResourcePath`, optional + Path to the repository root this `Registry` will manage. + + Returns + ------- + registry : `Registry` + A new `Registry` instance. + + Notes + ----- + This method is for backward compatibility only, until all clients + migrate to use new `~lsst.daf.butler.registry.RegistryFactory` factory + class. Regular clients of registry class do not use this method, it is + only used by tests in multiple packages. + """ + from ._registry_factory import RegistryFactory + + return RegistryFactory(config).create_from_config(dimensionConfig, butlerRoot) + @abstractmethod def isWriteable(self) -> bool: """Return `True` if this registry allows write operations, and `False` diff --git a/python/lsst/daf/butler/registry/_registry_factory.py b/python/lsst/daf/butler/registry/_registry_factory.py new file mode 100644 index 0000000000..0f54c9545c --- /dev/null +++ b/python/lsst/daf/butler/registry/_registry_factory.py @@ -0,0 +1,128 @@ +# This file is part of daf_butler. +# +# Developed for the LSST Data Management System. +# This product includes software developed by the LSST Project +# (http://www.lsst.org). +# See the COPYRIGHT file at the top-level directory of this distribution +# for details of code ownership. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +from __future__ import annotations + +__all__ = ("RegistryFactory",) + +from typing import TYPE_CHECKING + +from lsst.resources import ResourcePathExpression +from lsst.utils import doImportType + +from ..core import Config, DimensionConfig +from ._butler_registry import ButlerRegistry +from ._config import RegistryConfig +from ._defaults import RegistryDefaults + +if TYPE_CHECKING: + from .._butlerConfig import ButlerConfig + + +class RegistryFactory: + """Interface for creating and initializing Registry instances. + + Parameters + ---------- + config : `RegistryConfig` or `str`, optional + Registry configuration, if missing then default configuration will + be loaded from registry.yaml. + + Notes + ----- + Each registry implementation can have its own constructor parameters. + The assumption is that an instance of a specific subclass will be + constructed from configuration using ``RegistryClass.fromConfig()`` or + ``RegistryClass.createFromConfig()``. + + This class will look for a ``cls`` entry in registry configuration object + (defaulting to ``SqlRegistry``), import that class, and call one of the + above methods on the imported class. + """ + + def __init__(self, config: ButlerConfig | RegistryConfig | Config | str | None): + if not isinstance(config, RegistryConfig): + if isinstance(config, (str, Config)) or config is None: + config = RegistryConfig(config) + else: + raise ValueError(f"Incompatible Registry configuration: {config}") + self._config = config + + # Default to the standard registry + registry_cls_name = config.get("cls", "lsst.daf.butler.registries.sql.SqlRegistry") + registry_cls = doImportType(registry_cls_name) + if not issubclass(registry_cls, ButlerRegistry): + raise TypeError( + f"Registry class obtained from config {registry_cls_name} is not a ButlerRegistry class." + ) + self._registry_cls = registry_cls + + def create_from_config( + self, + dimensionConfig: DimensionConfig | str | None = None, + butlerRoot: ResourcePathExpression | None = None, + ) -> ButlerRegistry: + """Create registry database and return `ButlerRegistry` instance. + + This method initializes database contents, database must be empty + prior to calling this method. + + Parameters + ---------- + dimensionConfig : `DimensionConfig` or `str`, optional + Dimensions configuration, if missing then default configuration + will be loaded from dimensions.yaml. + butlerRoot : convertible to `lsst.resources.ResourcePath`, optional + Path to the repository root this `Registry` will manage. + + Returns + ------- + registry : `ButlerRegistry` + A new `ButlerRegistry` instance. + """ + return self._registry_cls.createFromConfig(self._config, dimensionConfig, butlerRoot) + + def from_config( + self, + butlerRoot: ResourcePathExpression | None = None, + writeable: bool = True, + defaults: RegistryDefaults | None = None, + ) -> ButlerRegistry: + """Create `ButlerRegistry` subclass instance from ``config``. + + Registry database must be initialized prior to calling this method. + + Parameters + ---------- + butlerRoot : convertible to `lsst.resources.ResourcePath`, optional + Path to the repository root this `Registry` will manage. + writeable : `bool`, optional + If `True` (default) create a read-write connection to the database. + defaults : `~lsst.daf.butler.registry.RegistryDefaults`, optional + Default collection search path and/or output `~CollectionType.RUN` + collection. + + Returns + ------- + registry : `ButlerRegistry` (subclass) + A new `ButlerRegistry` subclass instance. + """ + return self._registry_cls.fromConfig(self._config, butlerRoot, writeable, defaults) diff --git a/tests/test_dimensions.py b/tests/test_dimensions.py index 4e17930bcb..1e6a268f10 100644 --- a/tests/test_dimensions.py +++ b/tests/test_dimensions.py @@ -45,7 +45,7 @@ TimespanDatabaseRepresentation, YamlRepoImportBackend, ) -from lsst.daf.butler.registry import ButlerRegistry, RegistryConfig +from lsst.daf.butler.registry import RegistryConfig, RegistryFactory DIMENSION_DATA_FILE = os.path.normpath( os.path.join(os.path.dirname(__file__), "data", "registry", "hsc-rc2-subset.yaml") @@ -64,7 +64,7 @@ def loadDimensionData() -> DataCoordinateSequence: # data and retreive it as a set of DataCoordinate objects. config = RegistryConfig() config["db"] = "sqlite://" - registry = ButlerRegistry.createFromConfig(config) + registry = RegistryFactory(config).create_from_config() with open(DIMENSION_DATA_FILE) as stream: backend = YamlRepoImportBackend(stream, registry) backend.register() diff --git a/tests/test_obscore.py b/tests/test_obscore.py index 6b1dbddd56..04890b6abd 100644 --- a/tests/test_obscore.py +++ b/tests/test_obscore.py @@ -38,7 +38,7 @@ StorageClassFactory, ) from lsst.daf.butler.registries.sql import SqlRegistry -from lsst.daf.butler.registry import ButlerRegistry, Registry, RegistryConfig +from lsst.daf.butler.registry import ButlerRegistry, Registry, RegistryConfig, RegistryFactory from lsst.daf.butler.registry.obscore import ( DatasetTypeConfig, ObsCoreConfig, @@ -67,7 +67,7 @@ def make_registry( ) -> ButlerRegistry: """Create new empty Registry.""" config = self.make_registry_config(collections, collection_type) - registry = ButlerRegistry.createFromConfig(config, butlerRoot=self.root) + registry = RegistryFactory(config).create_from_config(butlerRoot=self.root) self.initialize_registry(registry) return registry diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 5f139db579..7bcad02ac0 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -40,7 +40,7 @@ import sqlalchemy from lsst.daf.butler import Timespan, ddl -from lsst.daf.butler.registry import ButlerRegistry +from lsst.daf.butler.registry import ButlerRegistry, RegistryFactory from lsst.daf.butler.registry.databases.postgresql import PostgresqlDatabase, _RangeTimespanType from lsst.daf.butler.registry.tests import DatabaseTests, RegistryTests from lsst.daf.butler.tests.utils import makeTestTempDir, removeTestTempDir @@ -245,9 +245,9 @@ def makeRegistry(self, share_repo_with: ButlerRegistry | None = None) -> ButlerR config["db"] = self.server.url() config["namespace"] = namespace if share_repo_with is None: - return ButlerRegistry.createFromConfig(config) + return RegistryFactory(config).create_from_config() else: - return ButlerRegistry.fromConfig(config) + return RegistryFactory(config).from_config() class PostgresqlRegistryNameKeyCollMgrUUIDTestCase(PostgresqlRegistryTests, unittest.TestCase): diff --git a/tests/test_quantumBackedButler.py b/tests/test_quantumBackedButler.py index c027990f21..52a6306640 100644 --- a/tests/test_quantumBackedButler.py +++ b/tests/test_quantumBackedButler.py @@ -37,7 +37,7 @@ RegistryConfig, StorageClass, ) -from lsst.daf.butler.registry import ButlerRegistry +from lsst.daf.butler.registry import RegistryFactory from lsst.daf.butler.tests.utils import makeTestTempDir, removeTestTempDir from lsst.resources import ResourcePath @@ -55,7 +55,7 @@ def setUp(self) -> None: # Make a butler and import dimension definitions. registryConfig = RegistryConfig(self.config.get("registry")) - ButlerRegistry.createFromConfig(registryConfig, butlerRoot=self.root) + RegistryFactory(registryConfig).create_from_config(butlerRoot=self.root) self.butler = Butler(self.config, writeable=True, run="RUN") self.butler.import_(filename=os.path.join(TESTDIR, "data", "registry", "base.yaml")) diff --git a/tests/test_query_relations.py b/tests/test_query_relations.py index 120d777b87..971e03aeec 100644 --- a/tests/test_query_relations.py +++ b/tests/test_query_relations.py @@ -25,7 +25,7 @@ import re import unittest -from lsst.daf.butler.registry import ButlerRegistry, MissingSpatialOverlapError, RegistryConfig, queries +from lsst.daf.butler.registry import MissingSpatialOverlapError, RegistryConfig, RegistryFactory, queries from lsst.daf.butler.transfers import YamlRepoImportBackend TESTDIR = os.path.abspath(os.path.dirname(__file__)) @@ -52,7 +52,7 @@ class TestQueryRelationsTests(unittest.TestCase): def setUpClass(cls) -> None: config = RegistryConfig() config["db"] = "sqlite://" - cls.registry = ButlerRegistry.createFromConfig(config) + cls.registry = RegistryFactory(config).create_from_config() # We need just enough test data to have valid dimension records for # all of the dimensions we're concerned with, and we want to pick # values for each dimension that correspond to a spatiotemporal diff --git a/tests/test_simpleButler.py b/tests/test_simpleButler.py index 8178438da0..8e52990dbc 100644 --- a/tests/test_simpleButler.py +++ b/tests/test_simpleButler.py @@ -35,7 +35,7 @@ import astropy.time from lsst.daf.butler import Butler, ButlerConfig, CollectionType, DatasetId, DatasetRef, DatasetType, Timespan -from lsst.daf.butler.registry import ButlerRegistry, RegistryConfig, RegistryDefaults +from lsst.daf.butler.registry import RegistryConfig, RegistryDefaults, RegistryFactory from lsst.daf.butler.tests import DatastoreMock from lsst.daf.butler.tests.utils import makeTestTempDir, removeTestTempDir @@ -71,7 +71,7 @@ def makeButler(self, **kwargs: Any) -> Butler: # have to make a registry first registryConfig = RegistryConfig(config.get("registry")) - ButlerRegistry.createFromConfig(registryConfig) + RegistryFactory(registryConfig).create_from_config() butler = Butler(config, **kwargs) DatastoreMock.apply(butler) diff --git a/tests/test_sqlite.py b/tests/test_sqlite.py index 48467c09c2..ae1fa8ccfa 100644 --- a/tests/test_sqlite.py +++ b/tests/test_sqlite.py @@ -28,7 +28,7 @@ import sqlalchemy from lsst.daf.butler import ddl -from lsst.daf.butler.registry import ButlerRegistry +from lsst.daf.butler.registry import ButlerRegistry, RegistryFactory from lsst.daf.butler.registry.databases.sqlite import SqliteDatabase from lsst.daf.butler.registry.tests import DatabaseTests, RegistryTests from lsst.daf.butler.tests.utils import makeTestTempDir, removeTestTempDir @@ -204,9 +204,9 @@ def makeRegistry(self, share_repo_with: ButlerRegistry | None = None) -> ButlerR config = self.makeRegistryConfig() config["db"] = f"sqlite:///{filename}" if share_repo_with is None: - return ButlerRegistry.createFromConfig(config, butlerRoot=self.root) + return RegistryFactory(config).create_from_config(butlerRoot=self.root) else: - return ButlerRegistry.fromConfig(config, butlerRoot=self.root) + return RegistryFactory(config).from_config(butlerRoot=self.root) class SqliteFileRegistryNameKeyCollMgrUUIDTestCase(SqliteFileRegistryTests, unittest.TestCase): @@ -247,7 +247,7 @@ def makeRegistry(self, share_repo_with: ButlerRegistry | None = None) -> ButlerR return None config = self.makeRegistryConfig() config["db"] = "sqlite://" - return ButlerRegistry.createFromConfig(config) + return RegistryFactory(config).create_from_config() def testMissingAttributes(self): """Test for instantiating a registry against outdated schema which @@ -258,7 +258,7 @@ def testMissingAttributes(self): config = self.makeRegistryConfig() config["db"] = "sqlite://" with self.assertRaises(LookupError): - ButlerRegistry.fromConfig(config) + RegistryFactory(config).from_config() class SqliteMemoryRegistryNameKeyCollMgrUUIDTestCase(unittest.TestCase, SqliteMemoryRegistryTests): From 6e3cf8a16eaa36658bd3d78c79839f8fc9e0657f Mon Sep 17 00:00:00 2001 From: Andy Salnikov Date: Thu, 13 Jul 2023 15:04:25 -0700 Subject: [PATCH 5/6] Rename ButlerRegistry to _ButlerRegistry --- python/lsst/daf/butler/_butler.py | 6 ++-- python/lsst/daf/butler/registries/remote.py | 14 ++++---- python/lsst/daf/butler/registries/sql.py | 10 +++--- python/lsst/daf/butler/registry/__init__.py | 2 +- .../daf/butler/registry/_butler_registry.py | 34 +++++++++---------- .../daf/butler/registry/_registry_factory.py | 22 ++++++------ .../lsst/daf/butler/tests/_datasetsHelper.py | 4 +-- python/lsst/daf/butler/transfers/_context.py | 6 ++-- tests/test_obscore.py | 4 +-- tests/test_postgresql.py | 4 +-- tests/test_sqlite.py | 6 ++-- 11 files changed, 56 insertions(+), 56 deletions(-) diff --git a/python/lsst/daf/butler/_butler.py b/python/lsst/daf/butler/_butler.py index 34b7ab5109..0ff8fe74b5 100644 --- a/python/lsst/daf/butler/_butler.py +++ b/python/lsst/daf/butler/_butler.py @@ -78,7 +78,6 @@ from .core.repoRelocation import BUTLER_ROOT_TAG from .core.utils import transactional from .registry import ( - ButlerRegistry, CollectionType, ConflictingDefinitionError, DataIdError, @@ -88,6 +87,7 @@ RegistryConfig, RegistryDefaults, RegistryFactory, + _ButlerRegistry, ) from .transfers import RepoExportContext @@ -2649,9 +2649,9 @@ def dimensions(self) -> DimensionUniverse: # Docstring inherited. return self._registry.dimensions - _registry: ButlerRegistry + _registry: _ButlerRegistry """The object that manages dataset metadata and relationships - (`ButlerRegistry`). + (`_ButlerRegistry`). Most operations that don't involve reading or writing butler datasets are accessible only via `Registry` methods. diff --git a/python/lsst/daf/butler/registries/remote.py b/python/lsst/daf/butler/registries/remote.py index fbb93cc34e..f9f4959b6f 100644 --- a/python/lsst/daf/butler/registries/remote.py +++ b/python/lsst/daf/butler/registries/remote.py @@ -65,7 +65,7 @@ QueryDatasetsModel, QueryDimensionRecordsModel, ) -from ..registry import ButlerRegistry, CollectionSummary, CollectionType, RegistryConfig, RegistryDefaults +from ..registry import CollectionSummary, CollectionType, RegistryConfig, RegistryDefaults, _ButlerRegistry if TYPE_CHECKING: from .._butlerConfig import ButlerConfig @@ -73,7 +73,7 @@ from ..registry.interfaces import CollectionRecord, DatastoreRegistryBridgeManager -class RemoteRegistry(ButlerRegistry): +class RemoteRegistry(_ButlerRegistry): """Registry that can talk to a remote Butler server. Parameters @@ -91,8 +91,8 @@ def createFromConfig( config: RegistryConfig | str | None = None, dimensionConfig: DimensionConfig | str | None = None, butlerRoot: ResourcePathExpression | None = None, - ) -> ButlerRegistry: - """Create registry database and return `ButlerRegistry` instance. + ) -> _ButlerRegistry: + """Create registry database and return `_ButlerRegistry` instance. A remote registry can not create a registry database. Calling this method will raise an exception. @@ -106,7 +106,7 @@ def fromConfig( butlerRoot: ResourcePathExpression | None = None, writeable: bool = True, defaults: RegistryDefaults | None = None, - ) -> ButlerRegistry: + ) -> _ButlerRegistry: # Docstring inherited from lsst.daf.butler.registry.Registry config = cls.forceRegistryConfig(config) config.replaceRoot(butlerRoot) @@ -162,8 +162,8 @@ def isWriteable(self) -> bool: # Can be used to prevent any PUTs to server return self._writeable - def copy(self, defaults: RegistryDefaults | None = None) -> ButlerRegistry: - # Docstring inherited from lsst.daf.butler.registry.ButlerRegistry + def copy(self, defaults: RegistryDefaults | None = None) -> _ButlerRegistry: + # Docstring inherited from lsst.daf.butler.registry._ButlerRegistry if defaults is None: # No need to copy, because `RegistryDefaults` is immutable; we # effectively copy on write. diff --git a/python/lsst/daf/butler/registries/sql.py b/python/lsst/daf/butler/registries/sql.py index 04c987be76..dc112eeafb 100644 --- a/python/lsst/daf/butler/registries/sql.py +++ b/python/lsst/daf/butler/registries/sql.py @@ -61,7 +61,6 @@ from ..core.utils import transactional from ..registry import ( ArgumentError, - ButlerRegistry, CollectionExpressionError, CollectionSummary, CollectionType, @@ -76,6 +75,7 @@ RegistryConfig, RegistryConsistencyError, RegistryDefaults, + _ButlerRegistry, queries, ) from ..registry.interfaces import ChainedCollectionRecord, RunRecord @@ -96,7 +96,7 @@ _LOG = logging.getLogger(__name__) -class SqlRegistry(ButlerRegistry): +class SqlRegistry(_ButlerRegistry): """Registry implementation based on SQLAlchemy. Parameters @@ -121,7 +121,7 @@ def createFromConfig( config: RegistryConfig | str | None = None, dimensionConfig: DimensionConfig | str | None = None, butlerRoot: ResourcePathExpression | None = None, - ) -> ButlerRegistry: + ) -> _ButlerRegistry: """Create registry database and return `SqlRegistry` instance. This method initializes database contents, database must be empty @@ -168,7 +168,7 @@ def fromConfig( butlerRoot: ResourcePathExpression | None = None, writeable: bool = True, defaults: RegistryDefaults | None = None, - ) -> ButlerRegistry: + ) -> _ButlerRegistry: """Create `Registry` subclass instance from `config`. Registry database must be initialized prior to calling this method. @@ -225,7 +225,7 @@ def isWriteable(self) -> bool: # Docstring inherited from lsst.daf.butler.registry.Registry return self._db.isWriteable() - def copy(self, defaults: RegistryDefaults | None = None) -> ButlerRegistry: + def copy(self, defaults: RegistryDefaults | None = None) -> _ButlerRegistry: # Docstring inherited from lsst.daf.butler.registry.Registry if defaults is None: # No need to copy, because `RegistryDefaults` is immutable; we diff --git a/python/lsst/daf/butler/registry/__init__.py b/python/lsst/daf/butler/registry/__init__.py index 6151e93c24..c5819d0a24 100644 --- a/python/lsst/daf/butler/registry/__init__.py +++ b/python/lsst/daf/butler/registry/__init__.py @@ -20,7 +20,7 @@ # along with this program. If not, see . from . import interfaces, managers, queries, wildcards -from ._butler_registry import ButlerRegistry +from ._butler_registry import _ButlerRegistry from ._collection_summary import * from ._collectionType import * from ._config import * diff --git a/python/lsst/daf/butler/registry/_butler_registry.py b/python/lsst/daf/butler/registry/_butler_registry.py index 8a6d3bb633..f0150f320e 100644 --- a/python/lsst/daf/butler/registry/_butler_registry.py +++ b/python/lsst/daf/butler/registry/_butler_registry.py @@ -21,7 +21,7 @@ from __future__ import annotations -__all__ = ("ButlerRegistry",) +__all__ = ("_ButlerRegistry",) from abc import abstractmethod from typing import TYPE_CHECKING @@ -38,7 +38,7 @@ from .interfaces import CollectionRecord, DatastoreRegistryBridgeManager -class ButlerRegistry(Registry): +class _ButlerRegistry(Registry): """Registry interface extended with methods used by Butler implementation. Each registry implementation can have its own constructor parameters. @@ -84,8 +84,8 @@ def createFromConfig( config: RegistryConfig | str | None = None, dimensionConfig: DimensionConfig | str | None = None, butlerRoot: ResourcePathExpression | None = None, - ) -> ButlerRegistry: - """Create registry database and return `ButlerRegistry` instance. + ) -> _ButlerRegistry: + """Create registry database and return `_ButlerRegistry` instance. This method initializes database contents, database must be empty prior to calling this method. @@ -103,12 +103,12 @@ def createFromConfig( Returns ------- - registry : `ButlerRegistry` - A new `ButlerRegistry` instance. + registry : `_ButlerRegistry` + A new `_ButlerRegistry` instance. Notes ----- - This class will determine the concrete `ButlerRegistry` subclass to + This class will determine the concrete `_ButlerRegistry` subclass to use from configuration. Each subclass should implement this method even if it can not create a registry. """ @@ -122,8 +122,8 @@ def fromConfig( butlerRoot: ResourcePathExpression | None = None, writeable: bool = True, defaults: RegistryDefaults | None = None, - ) -> ButlerRegistry: - """Create `ButlerRegistry` subclass instance from ``config``. + ) -> _ButlerRegistry: + """Create `_ButlerRegistry` subclass instance from ``config``. Registry database must be initialized prior to calling this method. @@ -141,12 +141,12 @@ def fromConfig( Returns ------- - registry : `ButlerRegistry` (subclass) - A new `ButlerRegistry` subclass instance. + registry : `_ButlerRegistry` (subclass) + A new `_ButlerRegistry` subclass instance. Notes ----- - This class will determine the concrete `ButlerRegistry` subclass to + This class will determine the concrete `_ButlerRegistry` subclass to use from configuration. Each subclass should implement this method. """ # The base class implementation should trampoline to the correct @@ -156,9 +156,9 @@ def fromConfig( raise NotImplementedError() @abstractmethod - def copy(self, defaults: RegistryDefaults | None = None) -> ButlerRegistry: - """Create a new `ButlerRegistry` backed by the same data repository and - connection as this one, but independent defaults. + def copy(self, defaults: RegistryDefaults | None = None) -> _ButlerRegistry: + """Create a new `_ButlerRegistry` backed by the same data repository + and connection as this one, but independent defaults. Parameters ---------- @@ -169,8 +169,8 @@ def copy(self, defaults: RegistryDefaults | None = None) -> ButlerRegistry: Returns ------- - copy : `ButlerRegistry` - A new `ButlerRegistry` instance with its own defaults. + copy : `_ButlerRegistry` + A new `_ButlerRegistry` instance with its own defaults. Notes ----- diff --git a/python/lsst/daf/butler/registry/_registry_factory.py b/python/lsst/daf/butler/registry/_registry_factory.py index 0f54c9545c..9f2c727820 100644 --- a/python/lsst/daf/butler/registry/_registry_factory.py +++ b/python/lsst/daf/butler/registry/_registry_factory.py @@ -29,7 +29,7 @@ from lsst.utils import doImportType from ..core import Config, DimensionConfig -from ._butler_registry import ButlerRegistry +from ._butler_registry import _ButlerRegistry from ._config import RegistryConfig from ._defaults import RegistryDefaults @@ -69,9 +69,9 @@ def __init__(self, config: ButlerConfig | RegistryConfig | Config | str | None): # Default to the standard registry registry_cls_name = config.get("cls", "lsst.daf.butler.registries.sql.SqlRegistry") registry_cls = doImportType(registry_cls_name) - if not issubclass(registry_cls, ButlerRegistry): + if not issubclass(registry_cls, _ButlerRegistry): raise TypeError( - f"Registry class obtained from config {registry_cls_name} is not a ButlerRegistry class." + f"Registry class obtained from config {registry_cls_name} is not a _ButlerRegistry class." ) self._registry_cls = registry_cls @@ -79,8 +79,8 @@ def create_from_config( self, dimensionConfig: DimensionConfig | str | None = None, butlerRoot: ResourcePathExpression | None = None, - ) -> ButlerRegistry: - """Create registry database and return `ButlerRegistry` instance. + ) -> _ButlerRegistry: + """Create registry database and return `_ButlerRegistry` instance. This method initializes database contents, database must be empty prior to calling this method. @@ -95,8 +95,8 @@ def create_from_config( Returns ------- - registry : `ButlerRegistry` - A new `ButlerRegistry` instance. + registry : `_ButlerRegistry` + A new `_ButlerRegistry` instance. """ return self._registry_cls.createFromConfig(self._config, dimensionConfig, butlerRoot) @@ -105,8 +105,8 @@ def from_config( butlerRoot: ResourcePathExpression | None = None, writeable: bool = True, defaults: RegistryDefaults | None = None, - ) -> ButlerRegistry: - """Create `ButlerRegistry` subclass instance from ``config``. + ) -> _ButlerRegistry: + """Create `_ButlerRegistry` subclass instance from ``config``. Registry database must be initialized prior to calling this method. @@ -122,7 +122,7 @@ def from_config( Returns ------- - registry : `ButlerRegistry` (subclass) - A new `ButlerRegistry` subclass instance. + registry : `_ButlerRegistry` (subclass) + A new `_ButlerRegistry` subclass instance. """ return self._registry_cls.fromConfig(self._config, butlerRoot, writeable, defaults) diff --git a/python/lsst/daf/butler/tests/_datasetsHelper.py b/python/lsst/daf/butler/tests/_datasetsHelper.py index 562cdc2b70..c7be535714 100644 --- a/python/lsst/daf/butler/tests/_datasetsHelper.py +++ b/python/lsst/daf/butler/tests/_datasetsHelper.py @@ -38,7 +38,7 @@ if TYPE_CHECKING: from lsst.daf.butler import Config, DatasetId, Datastore, Dimension, DimensionGraph - from lsst.daf.butler.registry import ButlerRegistry + from lsst.daf.butler.registry import _ButlerRegistry class DatasetTestHelper: @@ -102,7 +102,7 @@ class DatastoreTestHelper: datastoreType: type[Datastore] configFile: str - def setUpDatastoreTests(self, registryClass: type[ButlerRegistry], configClass: type[Config]) -> None: + def setUpDatastoreTests(self, registryClass: type[_ButlerRegistry], configClass: type[Config]) -> None: """Shared setUp code for all Datastore tests.""" self.registry = registryClass() self.config = configClass(self.configFile) diff --git a/python/lsst/daf/butler/transfers/_context.py b/python/lsst/daf/butler/transfers/_context.py index aadc0f1445..8e71516316 100644 --- a/python/lsst/daf/butler/transfers/_context.py +++ b/python/lsst/daf/butler/transfers/_context.py @@ -38,7 +38,7 @@ DimensionRecord, FileDataset, ) -from ..registry import ButlerRegistry, CollectionType +from ..registry import CollectionType, _ButlerRegistry from ..registry.interfaces import ChainedCollectionRecord, CollectionRecord from ._interfaces import RepoExportBackend @@ -58,7 +58,7 @@ class RepoExportContext: Parameters ---------- - registry : `ButlerRegistry` + registry : `_ButlerRegistry` Registry to export from. datastore : `Datastore` Datastore to export from. @@ -73,7 +73,7 @@ class RepoExportContext: def __init__( self, - registry: ButlerRegistry, + registry: _ButlerRegistry, datastore: Datastore, backend: RepoExportBackend, *, diff --git a/tests/test_obscore.py b/tests/test_obscore.py index 04890b6abd..d1ef99a2d5 100644 --- a/tests/test_obscore.py +++ b/tests/test_obscore.py @@ -38,7 +38,7 @@ StorageClassFactory, ) from lsst.daf.butler.registries.sql import SqlRegistry -from lsst.daf.butler.registry import ButlerRegistry, Registry, RegistryConfig, RegistryFactory +from lsst.daf.butler.registry import Registry, RegistryConfig, RegistryFactory, _ButlerRegistry from lsst.daf.butler.registry.obscore import ( DatasetTypeConfig, ObsCoreConfig, @@ -64,7 +64,7 @@ class ObsCoreTests(TestCaseMixin): def make_registry( self, collections: list[str] | None = None, collection_type: str | None = None - ) -> ButlerRegistry: + ) -> _ButlerRegistry: """Create new empty Registry.""" config = self.make_registry_config(collections, collection_type) registry = RegistryFactory(config).create_from_config(butlerRoot=self.root) diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 7bcad02ac0..c56dcc309a 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -40,7 +40,7 @@ import sqlalchemy from lsst.daf.butler import Timespan, ddl -from lsst.daf.butler.registry import ButlerRegistry, RegistryFactory +from lsst.daf.butler.registry import RegistryFactory, _ButlerRegistry from lsst.daf.butler.registry.databases.postgresql import PostgresqlDatabase, _RangeTimespanType from lsst.daf.butler.registry.tests import DatabaseTests, RegistryTests from lsst.daf.butler.tests.utils import makeTestTempDir, removeTestTempDir @@ -236,7 +236,7 @@ def tearDownClass(cls): def getDataDir(cls) -> str: return os.path.normpath(os.path.join(os.path.dirname(__file__), "data", "registry")) - def makeRegistry(self, share_repo_with: ButlerRegistry | None = None) -> ButlerRegistry: + def makeRegistry(self, share_repo_with: _ButlerRegistry | None = None) -> _ButlerRegistry: if share_repo_with is None: namespace = f"namespace_{secrets.token_hex(8).lower()}" else: diff --git a/tests/test_sqlite.py b/tests/test_sqlite.py index ae1fa8ccfa..386ba1e6fd 100644 --- a/tests/test_sqlite.py +++ b/tests/test_sqlite.py @@ -28,7 +28,7 @@ import sqlalchemy from lsst.daf.butler import ddl -from lsst.daf.butler.registry import ButlerRegistry, RegistryFactory +from lsst.daf.butler.registry import RegistryFactory, _ButlerRegistry from lsst.daf.butler.registry.databases.sqlite import SqliteDatabase from lsst.daf.butler.registry.tests import DatabaseTests, RegistryTests from lsst.daf.butler.tests.utils import makeTestTempDir, removeTestTempDir @@ -196,7 +196,7 @@ def tearDown(self): def getDataDir(cls) -> str: return os.path.normpath(os.path.join(os.path.dirname(__file__), "data", "registry")) - def makeRegistry(self, share_repo_with: ButlerRegistry | None = None) -> ButlerRegistry: + def makeRegistry(self, share_repo_with: _ButlerRegistry | None = None) -> _ButlerRegistry: if share_repo_with is None: _, filename = tempfile.mkstemp(dir=self.root, suffix=".sqlite3") else: @@ -242,7 +242,7 @@ class SqliteMemoryRegistryTests(RegistryTests): def getDataDir(cls) -> str: return os.path.normpath(os.path.join(os.path.dirname(__file__), "data", "registry")) - def makeRegistry(self, share_repo_with: ButlerRegistry | None = None) -> ButlerRegistry | None: + def makeRegistry(self, share_repo_with: _ButlerRegistry | None = None) -> _ButlerRegistry | None: if share_repo_with is not None: return None config = self.makeRegistryConfig() From 5b07c4deeac256f91bb14a0e7fa1f40747bf6b77 Mon Sep 17 00:00:00 2001 From: Andy Salnikov Date: Thu, 13 Jul 2023 15:08:47 -0700 Subject: [PATCH 6/6] Rename RegistryFactory to _RegistryFactory --- python/lsst/daf/butler/_butler.py | 6 +++--- python/lsst/daf/butler/registry/_registry.py | 6 +++--- python/lsst/daf/butler/registry/_registry_factory.py | 4 ++-- tests/test_dimensions.py | 4 ++-- tests/test_obscore.py | 4 ++-- tests/test_postgresql.py | 6 +++--- tests/test_quantumBackedButler.py | 4 ++-- tests/test_query_relations.py | 4 ++-- tests/test_simpleButler.py | 4 ++-- tests/test_sqlite.py | 10 +++++----- 10 files changed, 26 insertions(+), 26 deletions(-) diff --git a/python/lsst/daf/butler/_butler.py b/python/lsst/daf/butler/_butler.py index 0ff8fe74b5..2b04691631 100644 --- a/python/lsst/daf/butler/_butler.py +++ b/python/lsst/daf/butler/_butler.py @@ -86,8 +86,8 @@ Registry, RegistryConfig, RegistryDefaults, - RegistryFactory, _ButlerRegistry, + _RegistryFactory, ) from .transfers import RepoExportContext @@ -225,7 +225,7 @@ def __init__( butlerRoot = self._config.configDir if writeable is None: writeable = run is not None - self._registry = RegistryFactory(self._config).from_config( + self._registry = _RegistryFactory(self._config).from_config( butlerRoot=butlerRoot, writeable=writeable, defaults=defaults ) self._datastore = Datastore.fromConfig( @@ -463,7 +463,7 @@ def makeRepo( # Create Registry and populate tables registryConfig = RegistryConfig(config.get("registry")) dimensionConfig = DimensionConfig(dimensionConfig) - RegistryFactory(registryConfig).create_from_config( + _RegistryFactory(registryConfig).create_from_config( dimensionConfig=dimensionConfig, butlerRoot=root_uri ) diff --git a/python/lsst/daf/butler/registry/_registry.py b/python/lsst/daf/butler/registry/_registry.py index 99c2080fdb..b97c62a1d4 100644 --- a/python/lsst/daf/butler/registry/_registry.py +++ b/python/lsst/daf/butler/registry/_registry.py @@ -106,13 +106,13 @@ def createFromConfig( Notes ----- This method is for backward compatibility only, until all clients - migrate to use new `~lsst.daf.butler.registry.RegistryFactory` factory + migrate to use new `~lsst.daf.butler.registry._RegistryFactory` factory class. Regular clients of registry class do not use this method, it is only used by tests in multiple packages. """ - from ._registry_factory import RegistryFactory + from ._registry_factory import _RegistryFactory - return RegistryFactory(config).create_from_config(dimensionConfig, butlerRoot) + return _RegistryFactory(config).create_from_config(dimensionConfig, butlerRoot) @abstractmethod def isWriteable(self) -> bool: diff --git a/python/lsst/daf/butler/registry/_registry_factory.py b/python/lsst/daf/butler/registry/_registry_factory.py index 9f2c727820..74bb9291df 100644 --- a/python/lsst/daf/butler/registry/_registry_factory.py +++ b/python/lsst/daf/butler/registry/_registry_factory.py @@ -21,7 +21,7 @@ from __future__ import annotations -__all__ = ("RegistryFactory",) +__all__ = ("_RegistryFactory",) from typing import TYPE_CHECKING @@ -37,7 +37,7 @@ from .._butlerConfig import ButlerConfig -class RegistryFactory: +class _RegistryFactory: """Interface for creating and initializing Registry instances. Parameters diff --git a/tests/test_dimensions.py b/tests/test_dimensions.py index 1e6a268f10..291ede05c3 100644 --- a/tests/test_dimensions.py +++ b/tests/test_dimensions.py @@ -45,7 +45,7 @@ TimespanDatabaseRepresentation, YamlRepoImportBackend, ) -from lsst.daf.butler.registry import RegistryConfig, RegistryFactory +from lsst.daf.butler.registry import RegistryConfig, _RegistryFactory DIMENSION_DATA_FILE = os.path.normpath( os.path.join(os.path.dirname(__file__), "data", "registry", "hsc-rc2-subset.yaml") @@ -64,7 +64,7 @@ def loadDimensionData() -> DataCoordinateSequence: # data and retreive it as a set of DataCoordinate objects. config = RegistryConfig() config["db"] = "sqlite://" - registry = RegistryFactory(config).create_from_config() + registry = _RegistryFactory(config).create_from_config() with open(DIMENSION_DATA_FILE) as stream: backend = YamlRepoImportBackend(stream, registry) backend.register() diff --git a/tests/test_obscore.py b/tests/test_obscore.py index d1ef99a2d5..bf213496f3 100644 --- a/tests/test_obscore.py +++ b/tests/test_obscore.py @@ -38,7 +38,7 @@ StorageClassFactory, ) from lsst.daf.butler.registries.sql import SqlRegistry -from lsst.daf.butler.registry import Registry, RegistryConfig, RegistryFactory, _ButlerRegistry +from lsst.daf.butler.registry import Registry, RegistryConfig, _ButlerRegistry, _RegistryFactory from lsst.daf.butler.registry.obscore import ( DatasetTypeConfig, ObsCoreConfig, @@ -67,7 +67,7 @@ def make_registry( ) -> _ButlerRegistry: """Create new empty Registry.""" config = self.make_registry_config(collections, collection_type) - registry = RegistryFactory(config).create_from_config(butlerRoot=self.root) + registry = _RegistryFactory(config).create_from_config(butlerRoot=self.root) self.initialize_registry(registry) return registry diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index c56dcc309a..b1e5d573bc 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -40,7 +40,7 @@ import sqlalchemy from lsst.daf.butler import Timespan, ddl -from lsst.daf.butler.registry import RegistryFactory, _ButlerRegistry +from lsst.daf.butler.registry import _ButlerRegistry, _RegistryFactory from lsst.daf.butler.registry.databases.postgresql import PostgresqlDatabase, _RangeTimespanType from lsst.daf.butler.registry.tests import DatabaseTests, RegistryTests from lsst.daf.butler.tests.utils import makeTestTempDir, removeTestTempDir @@ -245,9 +245,9 @@ def makeRegistry(self, share_repo_with: _ButlerRegistry | None = None) -> _Butle config["db"] = self.server.url() config["namespace"] = namespace if share_repo_with is None: - return RegistryFactory(config).create_from_config() + return _RegistryFactory(config).create_from_config() else: - return RegistryFactory(config).from_config() + return _RegistryFactory(config).from_config() class PostgresqlRegistryNameKeyCollMgrUUIDTestCase(PostgresqlRegistryTests, unittest.TestCase): diff --git a/tests/test_quantumBackedButler.py b/tests/test_quantumBackedButler.py index 52a6306640..486fe2113d 100644 --- a/tests/test_quantumBackedButler.py +++ b/tests/test_quantumBackedButler.py @@ -37,7 +37,7 @@ RegistryConfig, StorageClass, ) -from lsst.daf.butler.registry import RegistryFactory +from lsst.daf.butler.registry import _RegistryFactory from lsst.daf.butler.tests.utils import makeTestTempDir, removeTestTempDir from lsst.resources import ResourcePath @@ -55,7 +55,7 @@ def setUp(self) -> None: # Make a butler and import dimension definitions. registryConfig = RegistryConfig(self.config.get("registry")) - RegistryFactory(registryConfig).create_from_config(butlerRoot=self.root) + _RegistryFactory(registryConfig).create_from_config(butlerRoot=self.root) self.butler = Butler(self.config, writeable=True, run="RUN") self.butler.import_(filename=os.path.join(TESTDIR, "data", "registry", "base.yaml")) diff --git a/tests/test_query_relations.py b/tests/test_query_relations.py index 971e03aeec..096a7aa7b8 100644 --- a/tests/test_query_relations.py +++ b/tests/test_query_relations.py @@ -25,7 +25,7 @@ import re import unittest -from lsst.daf.butler.registry import MissingSpatialOverlapError, RegistryConfig, RegistryFactory, queries +from lsst.daf.butler.registry import MissingSpatialOverlapError, RegistryConfig, _RegistryFactory, queries from lsst.daf.butler.transfers import YamlRepoImportBackend TESTDIR = os.path.abspath(os.path.dirname(__file__)) @@ -52,7 +52,7 @@ class TestQueryRelationsTests(unittest.TestCase): def setUpClass(cls) -> None: config = RegistryConfig() config["db"] = "sqlite://" - cls.registry = RegistryFactory(config).create_from_config() + cls.registry = _RegistryFactory(config).create_from_config() # We need just enough test data to have valid dimension records for # all of the dimensions we're concerned with, and we want to pick # values for each dimension that correspond to a spatiotemporal diff --git a/tests/test_simpleButler.py b/tests/test_simpleButler.py index 8e52990dbc..87d8dc08d8 100644 --- a/tests/test_simpleButler.py +++ b/tests/test_simpleButler.py @@ -35,7 +35,7 @@ import astropy.time from lsst.daf.butler import Butler, ButlerConfig, CollectionType, DatasetId, DatasetRef, DatasetType, Timespan -from lsst.daf.butler.registry import RegistryConfig, RegistryDefaults, RegistryFactory +from lsst.daf.butler.registry import RegistryConfig, RegistryDefaults, _RegistryFactory from lsst.daf.butler.tests import DatastoreMock from lsst.daf.butler.tests.utils import makeTestTempDir, removeTestTempDir @@ -71,7 +71,7 @@ def makeButler(self, **kwargs: Any) -> Butler: # have to make a registry first registryConfig = RegistryConfig(config.get("registry")) - RegistryFactory(registryConfig).create_from_config() + _RegistryFactory(registryConfig).create_from_config() butler = Butler(config, **kwargs) DatastoreMock.apply(butler) diff --git a/tests/test_sqlite.py b/tests/test_sqlite.py index 386ba1e6fd..7081354234 100644 --- a/tests/test_sqlite.py +++ b/tests/test_sqlite.py @@ -28,7 +28,7 @@ import sqlalchemy from lsst.daf.butler import ddl -from lsst.daf.butler.registry import RegistryFactory, _ButlerRegistry +from lsst.daf.butler.registry import _ButlerRegistry, _RegistryFactory from lsst.daf.butler.registry.databases.sqlite import SqliteDatabase from lsst.daf.butler.registry.tests import DatabaseTests, RegistryTests from lsst.daf.butler.tests.utils import makeTestTempDir, removeTestTempDir @@ -204,9 +204,9 @@ def makeRegistry(self, share_repo_with: _ButlerRegistry | None = None) -> _Butle config = self.makeRegistryConfig() config["db"] = f"sqlite:///{filename}" if share_repo_with is None: - return RegistryFactory(config).create_from_config(butlerRoot=self.root) + return _RegistryFactory(config).create_from_config(butlerRoot=self.root) else: - return RegistryFactory(config).from_config(butlerRoot=self.root) + return _RegistryFactory(config).from_config(butlerRoot=self.root) class SqliteFileRegistryNameKeyCollMgrUUIDTestCase(SqliteFileRegistryTests, unittest.TestCase): @@ -247,7 +247,7 @@ def makeRegistry(self, share_repo_with: _ButlerRegistry | None = None) -> _Butle return None config = self.makeRegistryConfig() config["db"] = "sqlite://" - return RegistryFactory(config).create_from_config() + return _RegistryFactory(config).create_from_config() def testMissingAttributes(self): """Test for instantiating a registry against outdated schema which @@ -258,7 +258,7 @@ def testMissingAttributes(self): config = self.makeRegistryConfig() config["db"] = "sqlite://" with self.assertRaises(LookupError): - RegistryFactory(config).from_config() + _RegistryFactory(config).from_config() class SqliteMemoryRegistryNameKeyCollMgrUUIDTestCase(unittest.TestCase, SqliteMemoryRegistryTests):