Skip to content

Commit

Permalink
Make Butler._clone public as Butler.clone
Browse files Browse the repository at this point in the history
  • Loading branch information
dhirving committed Sep 13, 2024
1 parent 57cb9c8 commit 91556de
Show file tree
Hide file tree
Showing 12 changed files with 45 additions and 23 deletions.
1 change: 1 addition & 0 deletions doc/changes/DM-46298.feature.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Added `Butler.clone()`, which lets you make a copy of a Butler instance, optionally overriding default collections/run/data ID.
9 changes: 4 additions & 5 deletions python/lsst/daf/butler/_butler.py
Original file line number Diff line number Diff line change
Expand Up @@ -276,9 +276,8 @@ def from_config(
collection arguments.
"""
# DirectButler used to have a way to specify a "copy constructor" by
# passing the "butler" parameter to its constructor. This
# functionality has been moved out of the constructor into
# Butler._clone(), but the new interface is not public yet.
# passing the "butler" parameter to its constructor. This has
# been moved out of the constructor into Butler.clone().
butler = kwargs.pop("butler", None)
if butler is not None:
if not isinstance(butler, Butler):
Expand All @@ -287,7 +286,7 @@ def from_config(
raise TypeError(
"Cannot pass 'config', 'searchPaths', or 'writeable' arguments with 'butler' argument."
)
return butler._clone(collections=collections, run=run, inferDefaults=inferDefaults, dataId=kwargs)
return butler.clone(collections=collections, run=run, inferDefaults=inferDefaults, dataId=kwargs)

options = ButlerInstanceOptions(
collections=collections, run=run, writeable=writeable, inferDefaults=inferDefaults, kwargs=kwargs
Expand Down Expand Up @@ -1827,7 +1826,7 @@ def query_dimension_records(
raise EmptyQueryResultError(list(result.explain_no_results()))
return dimension_records

def _clone(
def clone(
self,
*,
collections: CollectionArgType | None | EllipsisType = ...,
Expand Down
2 changes: 1 addition & 1 deletion python/lsst/daf/butler/_labeled_butler_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ def _create_direct_butler_factory(config: ButlerConfig, preload_cache: bool) ->
def create_butler(access_token: str | None) -> Butler:
# Access token is ignored because DirectButler does not use Gafaelfawr
# authentication.
return butler._clone()
return butler.clone()

return create_butler

Expand Down
2 changes: 1 addition & 1 deletion python/lsst/daf/butler/direct_butler/_direct_butler.py
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ def create_from_config(
_LOG.error(f"Failed to instantiate Butler from config {config.configFile}.")
raise

def _clone(
def clone(
self,
*,
collections: CollectionArgType | None | EllipsisType = ...,
Expand Down
4 changes: 2 additions & 2 deletions python/lsst/daf/butler/registry/tests/_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -723,7 +723,7 @@ def testCollections(self):
"""Tests for registry methods that manage collections."""
butler = self.make_butler()
registry = butler.registry
other_registry = butler._clone().registry
other_registry = butler.clone().registry
self.load_data(butler, "base.yaml", "datasets.yaml")
run1 = "imported_g"
run2 = "imported_r"
Expand Down Expand Up @@ -975,7 +975,7 @@ def _do_collection_concurrency_test(

# Set up two registries pointing to the same DB
butler1 = self.make_butler()
butler2 = butler1._clone()
butler2 = butler1.clone()
registry1 = butler1._registry
assert isinstance(registry1, SqlRegistry)
registry2 = butler2._registry
Expand Down
2 changes: 1 addition & 1 deletion python/lsst/daf/butler/remote_butler/_remote_butler.py
Original file line number Diff line number Diff line change
Expand Up @@ -591,7 +591,7 @@ def _normalize_collections(self, collections: CollectionArgType | None) -> Colle
collections = self.collections
return convert_collection_arg_to_glob_string_list(collections)

def _clone(
def clone(
self,
*,
collections: CollectionArgType | None | EllipsisType = ...,
Expand Down
6 changes: 3 additions & 3 deletions python/lsst/daf/butler/tests/hybrid_butler.py
Original file line number Diff line number Diff line change
Expand Up @@ -329,18 +329,18 @@ def registry(self) -> Registry:
def query(self) -> AbstractContextManager[Query]:
return self._remote_butler.query()

def _clone(
def clone(
self,
*,
collections: CollectionArgType | None | EllipsisType = ...,
run: str | None | EllipsisType = ...,
inferDefaults: bool | EllipsisType = ...,
dataId: dict[str, str] | EllipsisType = ...,
) -> HybridButler:
remote_butler = self._remote_butler._clone(
remote_butler = self._remote_butler.clone(
collections=collections, run=run, inferDefaults=inferDefaults, dataId=dataId
)
direct_butler = self._direct_butler._clone(
direct_butler = self._direct_butler.clone(
collections=collections, run=run, inferDefaults=inferDefaults, dataId=dataId
)
return HybridButler(remote_butler, direct_butler)
Expand Down
2 changes: 1 addition & 1 deletion python/lsst/daf/butler/tests/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ def create_from_butler(
instance.
"""
self = cls.__new__(cls)
butler = butler._clone(run=self._DEFAULT_RUN, collections=[self._DEFAULT_TAG])
butler = butler.clone(run=self._DEFAULT_RUN, collections=[self._DEFAULT_TAG])
self._do_init(butler, butler_config_file, storageClassName)
return self

Expand Down
6 changes: 3 additions & 3 deletions tests/test_butler.py
Original file line number Diff line number Diff line change
Expand Up @@ -2113,7 +2113,7 @@ def create_butler(
self, run: str, storageClass: StorageClass | str, datasetTypeName: str
) -> tuple[DirectButler, DatasetType]:
butler, datasetType = super().create_butler(run, storageClass, datasetTypeName)
return butler._clone(run=run), datasetType
return butler.clone(run=run), datasetType


class InMemoryDatastoreButlerTestCase(ButlerTests, unittest.TestCase):
Expand All @@ -2138,7 +2138,7 @@ def create_butler(
self, run: str, storageClass: StorageClass | str, datasetTypeName: str
) -> tuple[DirectButler, DatasetType]:
butler, datasetType = super().create_butler(run, storageClass, datasetTypeName)
return butler._clone(run=run), datasetType
return butler.clone(run=run), datasetType


class ChainedDatastoreButlerTestCase(FileDatastoreButlerTests, unittest.TestCase):
Expand Down Expand Up @@ -2838,7 +2838,7 @@ def are_uris_equivalent(self, uri1: ResourcePath, uri2: ResourcePath) -> bool:
return uri1.scheme == uri2.scheme and uri1.netloc == uri2.netloc and uri1.path == uri2.path

def create_empty_butler(self, run: str | None = None, writeable: bool | None = None) -> Butler:
return self.server_instance.hybrid_butler._clone(run=run)
return self.server_instance.hybrid_butler.clone(run=run)

def remove_dataset_out_of_band(self, butler: Butler, ref: DatasetRef) -> None:
# Can't delete a file via S3 signed URLs, so we need to reach in
Expand Down
2 changes: 1 addition & 1 deletion tests/test_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ def test_get(self):
)

# Test get() by DataId with default collections.
butler_with_default_collection = self.butler._clone(collections="ingest/run")
butler_with_default_collection = self.butler.clone(collections="ingest/run")
default_collection_metric = butler_with_default_collection.get(dataset_type, dataId=data_id)
self.assertEqual(metric, default_collection_metric)

Expand Down
30 changes: 26 additions & 4 deletions tests/test_simpleButler.py
Original file line number Diff line number Diff line change
Expand Up @@ -771,33 +771,55 @@ def test_clone(self):
butler = self.makeButler(writeable=True)
butler.import_(filename=os.path.join(TESTDIR, "data", "registry", "base.yaml"))
butler.import_(filename=os.path.join(TESTDIR, "data", "registry", "datasets.yaml"))
butler.import_(filename=os.path.join(TESTDIR, "data", "registry", "spatial.yaml"))

# Original butler was created with the default arguments:
# collections = None
# run = None
# inferDefaults = True
# no explicit default data ID

clone1 = butler._clone(collections="imported_g")
# Collections can be overridden, and default data ID will be inferred
# from it.
clone1 = butler.clone(collections="imported_g")
self.assertEqual(clone1.registry.defaults.dataId, {"instrument": "Cam1"})
self.assertCountEqual(clone1.registry.defaults.collections, ["imported_g"])
self.assertIsNone(clone1.run)

clone2 = clone1._clone(inferDefaults=False)
# Disabling inferDefaults stops default data ID from being inferred
# from collections.
clone2 = clone1.clone(inferDefaults=False)
self.assertEqual(clone2.registry.defaults.dataId, {})
self.assertCountEqual(clone2.registry.defaults.collections, ["imported_g"])
self.assertIsNone(clone2.run)

clone3 = clone2._clone(run="imported_r")
# Setting a new run doesn't override explicitly-set collections.
clone3 = clone2.clone(run="imported_r")
self.assertEqual(clone3.registry.defaults.dataId, {})
self.assertCountEqual(clone3.registry.defaults.collections, ["imported_g"])
self.assertEqual(clone3.run, "imported_r")

clone4 = butler._clone(run="imported_r")
# Following the behavior of the Butler() constructor, run will populate
# collections if collections was None. Default data ID is inferred
# from the run collection.
clone4 = butler.clone(run="imported_r")
self.assertEqual(clone4.registry.defaults.dataId, {"instrument": "Cam1"})
self.assertCountEqual(clone4.registry.defaults.collections, ["imported_r"])
self.assertEqual(clone4.run, "imported_r")

# Explicitly set data ID is combined with inferred defaults from
# collections.
clone5 = clone4.clone(dataId={"skymap": "SkyMap1"})
self.assertEqual(clone5.registry.defaults.dataId, {"instrument": "Cam1", "skymap": "SkyMap1"})
self.assertCountEqual(clone5.registry.defaults.collections, ["imported_r"])
self.assertEqual(clone5.run, "imported_r")

# Disabling inferred defaults preserves explicitly set data ID
clone6 = clone5.clone(inferDefaults=False)
self.assertEqual(clone6.registry.defaults.dataId, {"skymap": "SkyMap1"})
self.assertCountEqual(clone5.registry.defaults.collections, ["imported_r"])
self.assertEqual(clone5.run, "imported_r")


class DirectSimpleButlerTestCase(SimpleButlerTests, unittest.TestCase):
"""Run tests against DirectButler implementation."""
Expand Down
2 changes: 1 addition & 1 deletion tests/test_sqlite.py
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ class ClonedSqliteFileRegistryNameKeyCollMgrUUIDTestCase(

def make_butler(self) -> Butler:
original = super().make_butler()
return original._clone()
return original.clone()


class SqliteFileRegistrySynthIntKeyCollMgrUUIDTestCase(SqliteFileRegistryTests, unittest.TestCase):
Expand Down

0 comments on commit 91556de

Please sign in to comment.