From 03c318ae30cfe1bb14115357c1eb78113622f575 Mon Sep 17 00:00:00 2001 From: Tim Jenness Date: Fri, 16 Jun 2023 16:47:25 -0700 Subject: [PATCH 1/8] Fix warning from test when using exists() with potential ref --- tests/test_butler.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_butler.py b/tests/test_butler.py index 3bcee7c635..dc6f4686a3 100644 --- a/tests/test_butler.py +++ b/tests/test_butler.py @@ -365,7 +365,10 @@ def runPutGetTest(self, storageClass: StorageClass, datasetTypeName: str) -> But # Now remove the dataset completely. butler.pruneDatasets([ref], purge=True, unstore=True) # Lookup with original args should still fail. - self.assertFalse(butler.exists(*args, collections=this_run)) + kwargs = {"collections": this_run} + if isinstance(args[0], DatasetRef): + kwargs = {} # Prevent warning from being issued. + self.assertFalse(butler.exists(*args, **kwargs)) # get() should still fail. with self.assertRaises(FileNotFoundError): butler.get(ref) From 521ddf768c42e3aaf6f4955b7104482a95bfc61e Mon Sep 17 00:00:00 2001 From: Tim Jenness Date: Fri, 16 Jun 2023 16:54:35 -0700 Subject: [PATCH 2/8] Remove test that relied on integer ID reuse on import --- tests/test_simpleButler.py | 28 +--------------------------- 1 file changed, 1 insertion(+), 27 deletions(-) diff --git a/tests/test_simpleButler.py b/tests/test_simpleButler.py index 345cb13b97..dc3db7fd0e 100644 --- a/tests/test_simpleButler.py +++ b/tests/test_simpleButler.py @@ -36,7 +36,7 @@ import astropy.time from lsst.daf.butler import Butler, ButlerConfig, CollectionType, DatasetRef, DatasetType, Registry, Timespan -from lsst.daf.butler.registry import ConflictingDefinitionError, RegistryConfig, RegistryDefaults +from lsst.daf.butler.registry import RegistryConfig, RegistryDefaults from lsst.daf.butler.tests import DatastoreMock from lsst.daf.butler.tests.utils import makeTestTempDir, removeTestTempDir @@ -193,32 +193,6 @@ def testImportTwice(self): [self.comparableRef(ref) for ref in datasets2], ) - def testDatasetImportReuseIds(self): - """Test for import that should preserve dataset IDs. - - This test assumes that dataset IDs in datasets YAML are different from - what auto-incremental insert would produce. - """ - if self.datasetsIdType is not int: - self.skipTest("This test can only work for UUIDs") - # Import data to play with. - butler = self.makeButler(writeable=True) - butler.import_(filename=os.path.join(TESTDIR, "data", "registry", "base.yaml")) - filename = os.path.join(TESTDIR, "data", "registry", self.datasetsImportFile) - butler.import_(filename=filename, reuseIds=True) - datasets = list(butler.registry.queryDatasets(..., collections=...)) - self.assertTrue(all(isinstance(ref.id, self.datasetsIdType) for ref in datasets)) - # IDs are copied from YAML, list needs to be updated if file contents - # is changed. - self.assertCountEqual( - [ref.id for ref in datasets], - [1001, 1002, 1003, 1010, 1020, 1030, 2001, 2002, 2003, 2010, 2020, 2030, 2040], - ) - - # Try once again, it will raise - with self.assertRaises(ConflictingDefinitionError): - butler.import_(filename=filename, reuseIds=True) - def testCollectionTransfers(self): """Test exporting and then importing collections of various types.""" # Populate a registry with some datasets. From 0de13a35d2ee87340e1d730d0572bfa0a80ff43e Mon Sep 17 00:00:00 2001 From: Tim Jenness Date: Fri, 16 Jun 2023 16:55:49 -0700 Subject: [PATCH 3/8] Remove skip option when we are never going to skip --- tests/test_simpleButler.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/test_simpleButler.py b/tests/test_simpleButler.py index dc3db7fd0e..4db9ee07a6 100644 --- a/tests/test_simpleButler.py +++ b/tests/test_simpleButler.py @@ -169,8 +169,6 @@ def testImportTwice(self): """Test exporting dimension records and datasets from a repo and then importing them all back in again twice. """ - if self.datasetsIdType is not uuid.UUID: - self.skipTest("This test can only work for UUIDs") # Import data to play with. butler1 = self.makeButler(writeable=True) butler1.import_(filename=os.path.join(TESTDIR, "data", "registry", "base.yaml")) From ac539a0a7325466b3d7a01315f72aaa259aa4954 Mon Sep 17 00:00:00 2001 From: Tim Jenness Date: Fri, 16 Jun 2023 16:56:59 -0700 Subject: [PATCH 4/8] Remove class properties that are the same as parent class in test --- tests/test_simpleButler.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/test_simpleButler.py b/tests/test_simpleButler.py index 4db9ee07a6..e6787a7419 100644 --- a/tests/test_simpleButler.py +++ b/tests/test_simpleButler.py @@ -665,11 +665,7 @@ class SimpleButlerMixedUUIDTestCase(SimpleButlerTestCase): loads datasets from YAML file with integer IDs. """ - datasetsManager = ( - "lsst.daf.butler.registry.datasets.byDimensions.ByDimensionsDatasetRecordStorageManagerUUID" - ) datasetsImportFile = "datasets.yaml" - datasetsIdType = uuid.UUID if __name__ == "__main__": From ea432119911a5aab504bdf1f59ebab789161042a Mon Sep 17 00:00:00 2001 From: Tim Jenness Date: Fri, 16 Jun 2023 16:58:41 -0700 Subject: [PATCH 5/8] Remove unused parameters from _importDatasets remote They were removed from the other registry implementations long ago. --- python/lsst/daf/butler/registries/remote.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/python/lsst/daf/butler/registries/remote.py b/python/lsst/daf/butler/registries/remote.py index 8b2443b197..5297da0011 100644 --- a/python/lsst/daf/butler/registries/remote.py +++ b/python/lsst/daf/butler/registries/remote.py @@ -359,8 +359,6 @@ def _importDatasets( self, datasets: Iterable[DatasetRef], expand: bool = True, - idGenerationMode: DatasetIdGenEnum = DatasetIdGenEnum.UNIQUE, - reuseIds: bool = False, ) -> list[DatasetRef]: # Docstring inherited from lsst.daf.butler.registry.Registry raise NotImplementedError() From a5b73f52bf0be1406be5018d5cf82a6472a677bb Mon Sep 17 00:00:00 2001 From: Tim Jenness Date: Tue, 20 Jun 2023 09:06:44 -0700 Subject: [PATCH 6/8] Rename test option to stop confusing the test system On python 3.11 the previous code gave a warning: tests/test_cliUtils.py::MWOptionDecoratorTest::test_option .../python3.11/unittest/case.py:678: DeprecationWarning: It is deprecated to return a value that is not None from a test case () return self.run(*args, **kwds) --- tests/test_cliUtils.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/test_cliUtils.py b/tests/test_cliUtils.py index b7efcb4e7b..af3422e5f6 100644 --- a/tests/test_cliUtils.py +++ b/tests/test_cliUtils.py @@ -241,22 +241,22 @@ def cli(things): class MWOptionDecoratorTest(unittest.TestCase): """Tests for the MWOptionDecorator class.""" - test_option = MWOptionDecorator("-t", "--test", multiple=True) + _test_option = MWOptionDecorator("-t", "--test", multiple=True) def testGetName(self): """Test getting the option name from the MWOptionDecorator.""" - self.assertEqual(self.test_option.name(), "test") + self.assertEqual(self._test_option.name(), "test") def testGetOpts(self): """Test getting the option flags from the MWOptionDecorator.""" - self.assertEqual(self.test_option.opts(), ["-t", "--test"]) + self.assertEqual(self._test_option.opts(), ["-t", "--test"]) def testUse(self): """Test using the MWOptionDecorator with a command.""" mock = MagicMock() @click.command() - @self.test_option() + @self._test_option() def cli(test): mock(test) @@ -271,7 +271,7 @@ def testOverride(self): mock = MagicMock() @click.command() - @self.test_option(multiple=False) + @self._test_option(multiple=False) def cli(test): mock(test) From ae6d760b50b3a731e7cd9eb74e68ee6a146ad3ac Mon Sep 17 00:00:00 2001 From: Tim Jenness Date: Tue, 20 Jun 2023 13:46:32 -0700 Subject: [PATCH 7/8] Remove the code that handles tests of different ID types Now always assume that we have one DatasetId type. --- tests/test_simpleButler.py | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/tests/test_simpleButler.py b/tests/test_simpleButler.py index e6787a7419..ba0b117320 100644 --- a/tests/test_simpleButler.py +++ b/tests/test_simpleButler.py @@ -26,7 +26,6 @@ import re import tempfile import unittest -import uuid from typing import Any try: @@ -35,7 +34,16 @@ np = None import astropy.time -from lsst.daf.butler import Butler, ButlerConfig, CollectionType, DatasetRef, DatasetType, Registry, Timespan +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.tests import DatastoreMock from lsst.daf.butler.tests.utils import makeTestTempDir, removeTestTempDir @@ -53,7 +61,6 @@ class SimpleButlerTestCase(unittest.TestCase): "lsst.daf.butler.registry.datasets.byDimensions.ByDimensionsDatasetRecordStorageManagerUUID" ) datasetsImportFile = "datasets-uuid.yaml" - datasetsIdType = uuid.UUID def setUp(self): self.root = makeTestTempDir(TESTDIR) @@ -73,11 +80,7 @@ def makeButler(self, **kwargs: Any) -> Butler: # have to make a registry first registryConfig = RegistryConfig(config.get("registry")) - if self.datasetsIdType is int: - with self.assertWarns(FutureWarning): - Registry.createFromConfig(registryConfig) - else: - Registry.createFromConfig(registryConfig) + Registry.createFromConfig(registryConfig) butler = Butler(config, **kwargs) DatastoreMock.apply(butler) @@ -158,8 +161,8 @@ def testDatasetTransfers(self): butler2.import_(filename=file.name) datasets1 = list(butler1.registry.queryDatasets(..., collections=...)) datasets2 = list(butler2.registry.queryDatasets(..., collections=...)) - self.assertTrue(all(isinstance(ref.id, self.datasetsIdType) for ref in datasets1)) - self.assertTrue(all(isinstance(ref.id, self.datasetsIdType) for ref in datasets2)) + self.assertTrue(all(isinstance(ref.id, DatasetId) for ref in datasets1)) + self.assertTrue(all(isinstance(ref.id, DatasetId) for ref in datasets2)) self.assertCountEqual( [self.comparableRef(ref) for ref in datasets1], [self.comparableRef(ref) for ref in datasets2], @@ -184,8 +187,8 @@ def testImportTwice(self): butler2.import_(filename=file.name) datasets1 = list(butler1.registry.queryDatasets(..., collections=...)) datasets2 = list(butler2.registry.queryDatasets(..., collections=...)) - self.assertTrue(all(isinstance(ref.id, self.datasetsIdType) for ref in datasets1)) - self.assertTrue(all(isinstance(ref.id, self.datasetsIdType) for ref in datasets2)) + self.assertTrue(all(isinstance(ref.id, DatasetId) for ref in datasets1)) + self.assertTrue(all(isinstance(ref.id, DatasetId) for ref in datasets2)) self.assertCountEqual( [self.comparableRef(ref) for ref in datasets1], [self.comparableRef(ref) for ref in datasets2], From 661015584b8c47bfa150f36d5fe95493fd976910 Mon Sep 17 00:00:00 2001 From: Tim Jenness Date: Tue, 20 Jun 2023 13:51:46 -0700 Subject: [PATCH 8/8] Fix numpydoc error --- python/lsst/daf/butler/_dataset_existence.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/lsst/daf/butler/_dataset_existence.py b/python/lsst/daf/butler/_dataset_existence.py index d957d183f4..68dcd02eac 100644 --- a/python/lsst/daf/butler/_dataset_existence.py +++ b/python/lsst/daf/butler/_dataset_existence.py @@ -91,7 +91,7 @@ def __bool__(self) -> bool: Returns ------- - exists : `bool + exists : `bool` Evaluates to `True` if the flags evaluate to either ``KNOWN`` or ``VERIFIED``. """