DM-14321: Make StorageClass pickleable (by making instances instead of subclasses)

python/lsst/daf/butler/core/storageClass.py

@@ -26,7 +26,6 @@
 from .utils import doImport, Singleton
 from .composites import CompositeAssembler
 from .config import Config
-from .mappingFactory import MappingFactory
 
 __all__ = ("StorageClass", "StorageClassFactory", "StorageClassConfig")
 
@@ -37,60 +36,66 @@ class StorageClassConfig(Config):
 
 class StorageClass:
     """Class describing how a label maps to a particular Python type.
-    """
-
-    # Components are fixed per class
-    _components = {}
-
-    # These are internal class attributes supporting lazy loading of concrete
-    # python types and functions from the string. The lazy loading is only done
-    # once the property is requested by an instance. The loading is fixed per
-    # class but class access to attribute is not supported.
 
-    # The names are defined when the class is constructed
-    _pytypeName = None
-    _assemblerClassName = None
-    name = None
-
-    # The types are created on demand and cached
-    # We set a default assembler so that a class is guaranteed to support
-    # something.
-    _pytype = None
-    _assembler = CompositeAssembler
+    Parameters
+    ----------
+    name : `str`
+        Name to use for this class.
+    pytype : `type`
+        Python type (or name of type) to associate with the `StorageClass`
+    components : `dict`, optional
+        `dict` mapping name of a component to another `StorageClass`.
+    assembler : `str`, optional
+        Fully qualified name of class supporting assembly and disassembly
+        of a `pytype` instance.
+    """
+    def __init__(self, name, pytype=None, components=None, assembler=None):
+        self.name = name
+        self._pytypeName = pytype
+        self._components = components if components is not None else {}
+        # if the assembler is not None also set it and clear the default assembler
+        if assembler is not None:
+            self._assemblerClassName = assembler
+            self._assembler = None
+        else:
+            # We set a default assembler so that a class is guaranteed to support
+            # something.
+            self._assemblerClassName = None
+            self._assembler = CompositeAssembler
+        # The types are created on demand and cached
+        self._pytype = None
 
     @property
     def components(self):
         """Component names mapped to associated `StorageClass`
         """
-        return type(self)._components
+        return self._components
 
     @property
     def pytype(self):
         """Python type associated with this `StorageClass`."""
-        cls = type(self)
-        if cls._pytype is not None:
-            return cls._pytype
+        if self._pytype is not None:
+            return self._pytype
         # Handle case where we did get a python type not string
-        if not isinstance(cls._pytypeName, str):
-            pytype = cls._pytypeName
-            cls._pytypeName = cls._pytypeName.__name__
-        elif hasattr(builtins, cls._pytypeName):
-            pytype = getattr(builtins, cls._pytypeName)
+        if not isinstance(self._pytypeName, str):
+            pytype = self._pytypeName
+            self._pytypeName = self._pytypeName.__name__
+        elif hasattr(builtins, self._pytypeName):
+            pytype = getattr(builtins, self._pytypeName)
         else:
-            pytype = doImport(cls._pytypeName)
-        cls._pytype = pytype
-        return cls._pytype
+            pytype = doImport(self._pytypeName)
+        self._pytype = pytype
+        return self._pytype
 
     @property
     def assemblerClass(self):
         """Class to use to (dis)assemble an object from components."""
-        cls = type(self)
-        if cls._assembler is not None:
-            return cls._assembler
-        if cls._assemblerClassName is None:
+        if self._assembler is not None:
+            return self._assembler
+        if self._assemblerClassName is None:
             return None
-        cls._assembler = doImport(cls._assemblerClassName)
-        return cls._assembler
+        self._assembler = doImport(self._assemblerClassName)
+        return self._assembler
 
     def assembler(self):
         """Return an instance of an assembler.
@@ -99,10 +104,10 @@ def assembler(self):
         -------
         assembler : `CompositeAssembler`
             Instance of the assembler associated with this `StorageClass`.
-            Assembler is constructed with a new instance of this
-            `StorageClass`.
+            Assembler is constructed with this `StorageClass`.
         """
-        return self.assemblerClass(storageClass=type(self)())
+        cls = self.assemblerClass
+        return cls(storageClass=self)
 
     def validateInstance(self, instance):
         """Check that the supplied Python object has the expected Python type
@@ -127,40 +132,6 @@ def __hash__(self):
         return hash(self.name)
 
 
-def makeNewStorageClass(name, pytype=None, components=None, assembler=None):
-    """Create a new Python class as a subclass of `StorageClass`.
-
-    Parameters
-    ----------
-    name : `str`
-        Name to use for this class.
-    pytype : `type`
-        Python type (or name of type) to associate with the `StorageClass`
-    components : `dict`, optional
-        `dict` mapping name of a component to another `StorageClass`.
-    assembler : `str`, optional
-        Fully qualified name of class supporting assembly and disassembly
-        of a `pytype` instance.
-
-    Returns
-    -------
-    newtype : `StorageClass`
-        Newly created Python type.
-    """
-    if components is None:
-        components = {}
-
-    clsargs = {"name": name,
-               "_pytypeName": pytype,
-               "_components": components}
-    # if the assembler is not None also set it and clear the default assembler
-    if assembler is not None:
-        clsargs["_assemblerClassName"] = assembler
-        clsargs["_assembler"] = None
-
-    return type(name, (StorageClass,), clsargs)
-
-
 class StorageClassFactory(metaclass=Singleton):
     """Factory for `StorageClass` instances.
 
@@ -177,7 +148,7 @@ class StorageClassFactory(metaclass=Singleton):
     """
 
     def __init__(self, config=None):
-        self._mappingFactory = MappingFactory(StorageClass)
+        self._storageClasses = {}
         self._configs = []
 
         if config is not None:
@@ -244,7 +215,7 @@ def addFromConfig(self, config):
             storageClassKwargs["components"] = components
 
             # Create the new storage class and register it
-            newStorageClass = makeNewStorageClass(name, **storageClassKwargs)
+            newStorageClass = StorageClass(name, **storageClassKwargs)
             self.registerStorageClass(newStorageClass)
 
     def getStorageClass(self, storageClassName):
@@ -260,7 +231,7 @@ def getStorageClass(self, storageClassName):
         instance : `StorageClass`
             Instance of the correct `StorageClass`.
         """
-        return self._mappingFactory.getFromRegistry(storageClassName)
+        return self._storageClasses[storageClassName]
 
     def registerStorageClass(self, storageClass):
         """Store the `StorageClass` in the factory.
@@ -279,4 +250,4 @@ def registerStorageClass(self, storageClass):
             If a storage class has already been registered with
             storageClassName and the previous definition differs.
         """
-        self._mappingFactory.placeInRegistry(storageClass.name, storageClass)
+        self._storageClasses[storageClass.name] = storageClass

tests/test_datasets.py

@@ -24,7 +24,7 @@
 import lsst.utils.tests
 
 from lsst.daf.butler.core.datasets import DatasetType, DatasetRef
-from lsst.daf.butler.core.storageClass import makeNewStorageClass
+from lsst.daf.butler.core.storageClass import StorageClass
 
 """Tests for datasets module.
 """
@@ -42,16 +42,16 @@ def testConstructor(self):
         These can only be verified for a particular schema.
         """
         datasetTypeName = "test"
-        storageClass = makeNewStorageClass("test_StructuredData")()
+        storageClass = StorageClass("test_StructuredData")
         dataUnits = frozenset(("camera", "visit"))
         datasetType = DatasetType(datasetTypeName, dataUnits, storageClass)
         self.assertEqual(datasetType.name, datasetTypeName)
         self.assertEqual(datasetType.storageClass, storageClass)
         self.assertEqual(datasetType.dataUnits, dataUnits)
 
     def testEquality(self):
-        storageA = makeNewStorageClass("test_a")()
-        storageB = makeNewStorageClass("test_b")()
+        storageA = StorageClass("test_a")
+        storageB = StorageClass("test_b")
         self.assertEqual(DatasetType("a", ("UnitA", ), storageA,),
                          DatasetType("a", ("UnitA", ), storageA,))
         self.assertNotEqual(DatasetType("a", ("UnitA", ), storageA,),
@@ -74,8 +74,8 @@ def testHashability(self):
         """
         types = []
         unique = 0
-        storageC = makeNewStorageClass("test_c")()
-        storageD = makeNewStorageClass("test_d")()
+        storageC = StorageClass("test_c")
+        storageD = StorageClass("test_d")
         for name in ["a", "b"]:
             for storageClass in [storageC, storageD]:
                 for dataUnits in [("e", ), ("f", )]:
@@ -94,7 +94,7 @@ def testConstructor(self):
         """Test construction preserves values.
         """
         datasetTypeName = "test"
-        storageClass = makeNewStorageClass("testref_StructuredData")()
+        storageClass = StorageClass("testref_StructuredData")
         dataUnits = frozenset(("camera", "visit"))
         dataId = dict(camera="DummyCam", visit=42)
         datasetType = DatasetType(datasetTypeName, dataUnits, storageClass)

tests/test_formatter.py

@@ -75,10 +75,10 @@ def testRegistryWithStorageClass(self):
         """
         formatterTypeName = "lsst.daf.butler.formatters.yamlFormatter.YamlFormatter"
         storageClassName = "TestClass"
-        sc = storageClass.makeNewStorageClass(storageClassName, dict, None)
+        sc = storageClass.StorageClass(storageClassName, dict, None)
 
         # Store using an instance
-        self.factory.registerFormatter(sc(), formatterTypeName)
+        self.factory.registerFormatter(sc, formatterTypeName)
 
         # Retrieve using the class
         f = self.factory.getFormatter(sc)

tests/test_sqlRegistry.py

@@ -32,7 +32,7 @@
 from lsst.daf.butler.core.datasets import DatasetType
 from lsst.daf.butler.core.registry import Registry
 from lsst.daf.butler.registries.sqlRegistry import SqlRegistry
-from lsst.daf.butler.core.storageClass import makeNewStorageClass
+from lsst.daf.butler.core.storageClass import StorageClass
 
 """Tests for SqlRegistry.
 """
@@ -54,7 +54,7 @@ def testDatasetType(self):
         registry = Registry.fromConfig(self.configFile)
         # Check valid insert
         datasetTypeName = "test"
-        storageClass = makeNewStorageClass("testDatasetType")()
+        storageClass = StorageClass("testDatasetType")
         dataUnits = ("Camera", "Visit")
         inDatasetType = DatasetType(datasetTypeName, dataUnits, storageClass)
         registry.registerDatasetType(inDatasetType)
@@ -67,7 +67,7 @@ def testDatasetType(self):
 
         # Template can be None
         datasetTypeName = "testNoneTemplate"
-        storageClass = makeNewStorageClass("testDatasetType2")()
+        storageClass = StorageClass("testDatasetType2")
         dataUnits = ("Camera", "Visit")
         inDatasetType = DatasetType(datasetTypeName, dataUnits, storageClass)
         registry.registerDatasetType(inDatasetType)
@@ -77,7 +77,7 @@ def testDatasetType(self):
     def testDataset(self):
         registry = Registry.fromConfig(self.configFile)
         run = registry.makeRun(collection="test")
-        storageClass = makeNewStorageClass("testDataset")()
+        storageClass = StorageClass("testDataset")
         datasetType = DatasetType(name="testtype", dataUnits=("Camera",), storageClass=storageClass)
         registry.registerDatasetType(datasetType)
         ref = registry.addDataset(datasetType, dataId={"camera": "DummyCam"}, run=run)
@@ -86,7 +86,7 @@ def testDataset(self):
 
     def testComponents(self):
         registry = Registry.fromConfig(self.configFile)
-        storageClass = makeNewStorageClass("testComponents")()
+        storageClass = StorageClass("testComponents")
         parentDatasetType = DatasetType(name="parent", dataUnits=("Camera",), storageClass=storageClass)
         childDatasetType1 = DatasetType(name="child1", dataUnits=("Camera",), storageClass=storageClass)
         childDatasetType2 = DatasetType(name="child2", dataUnits=("Camera",), storageClass=storageClass)
@@ -152,7 +152,7 @@ def testExecution(self):
     def testQuantum(self):
         registry = Registry.fromConfig(self.configFile)
         run = registry.makeRun(collection="test")
-        storageClass = makeNewStorageClass("testQuantum")()
+        storageClass = StorageClass("testQuantum")
         # Make two predicted inputs
         datasetType1 = DatasetType(name="dst1", dataUnits=("Camera",), storageClass=storageClass)
         registry.registerDatasetType(datasetType1)
@@ -179,7 +179,7 @@ def testQuantum(self):
 
     def testStorageInfo(self):
         registry = Registry.fromConfig(self.configFile)
-        storageClass = makeNewStorageClass("testStorageInfo")()
+        storageClass = StorageClass("testStorageInfo")
         datasetType = DatasetType(name="test", dataUnits=("Camera",), storageClass=storageClass)
         registry.registerDatasetType(datasetType)
         run = registry.makeRun(collection="test")
@@ -201,7 +201,7 @@ def testStorageInfo(self):
 
     def testAssembler(self):
         registry = Registry.fromConfig(self.configFile)
-        storageClass = makeNewStorageClass("testAssembler")()
+        storageClass = StorageClass("testAssembler")
         datasetType = DatasetType(name="test", dataUnits=("Camera",), storageClass=storageClass)
         registry.registerDatasetType(datasetType)
         run = registry.makeRun(collection="test")
@@ -214,7 +214,7 @@ def testAssembler(self):
 
     def testFind(self):
         registry = Registry.fromConfig(self.configFile)
-        storageClass = makeNewStorageClass("testFind")()
+        storageClass = StorageClass("testFind")
         datasetType = DatasetType(name="dummytype", dataUnits=("Camera", "Visit"), storageClass=storageClass)
         registry.registerDatasetType(datasetType)
         collection = "test"
@@ -246,7 +246,7 @@ def testFind(self):
 
     def testCollections(self):
         registry = Registry.fromConfig(self.configFile)
-        storageClass = makeNewStorageClass("testCollections")()
+        storageClass = StorageClass("testCollections")
         datasetType = DatasetType(name="dummytype", dataUnits=("Camera", "Visit"), storageClass=storageClass)
         registry.registerDatasetType(datasetType)
         collection = "ingest"