DM-20915: Add str(Formatter) and force Formatter to require an arg for constructor

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

@@ -47,6 +47,22 @@ def __init__(self, location, storageClass, readStorageClass=None, parameters=Non
         self.storageClass = storageClass
         self.parameters = parameters
 
+    def __repr__(self):
+        optionals = {}
+        if self._readStorageClass is not None:
+            optionals["readStorageClass"] = self._readStorageClass
+        if self.parameters:
+            optionals["parameters"] = self.parameters
+
+        # order is preserved in the dict
+        options = ", ".join(f"{k}={v!r}" for k, v in optionals.items())
+
+        r = f"{self.__class__.__name__}({self.location!r}, {self.storageClass!r}"
+        if options:
+            r = r + ", " + options
+        r = r + ")"
+        return r
+
     @property
     def readStorageClass(self):
         """Storage class to use when reading. (`StorageClass`)

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

@@ -50,11 +50,17 @@ class Formatter(metaclass=ABCMeta):
     are supported (`frozenset`).
     """
 
-    def __init__(self, fileDescriptor=None):
-        if fileDescriptor is not None and not isinstance(fileDescriptor, FileDescriptor):
+    def __init__(self, fileDescriptor):
+        if not isinstance(fileDescriptor, FileDescriptor):
             raise TypeError("File descriptor must be a FileDescriptor")
         self._fileDescriptor = fileDescriptor
 
+    def __str__(self):
+        return f"{self.name()}@{self.fileDescriptor.location.path}"
+
+    def __repr__(self):
+        return f"{self.name()}({self.fileDescriptor!r})"
+
     @property
     def fileDescriptor(self):
         """FileDescriptor associated with this formatter

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

@@ -337,6 +337,11 @@ def __init__(self, datastoreRootUri, path):
     def __str__(self):
         return self.uri
 
+    def __repr__(self):
+        uri = self._datastoreRootUri.geturl()
+        path = self._path
+        return f"{self.__class__.__name__}({uri!r}, {path!r})"
+
     @property
     def uri(self):
         """URI string corresponding to fully-specified location in datastore.

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

@@ -77,10 +77,18 @@ def __init__(self, name=None, pytype=None, components=None, parameters=None, ass
         if assembler is None:
             assembler = self._cls_assembler
         self.name = name
-        self._pytypeName = pytype
+
         if pytype is None:
-            self._pytypeName = "object"
-            self._pytype = object
+            pytype = object
+
+        if not isinstance(pytype, str):
+            # Already have a type so store it and get the name
+            self._pytypeName = getFullTypeName(pytype)
+            self._pytype = pytype
+        else:
+            # Store the type name and defer loading of type
+            self._pytypeName = pytype
+
         self._components = components if components is not None else {}
         self._parameters = frozenset(parameters) if parameters is not None else frozenset()
         # if the assembler is not None also set it and clear the default
@@ -117,11 +125,8 @@ def pytype(self):
         """Python type associated with this `StorageClass`."""
         if self._pytype is not None:
             return self._pytype
-        # Handle case where we did get a python type not string
-        if not isinstance(self._pytypeName, str):
-            pytype = self._pytypeName
-            self._pytypeName = self._pytypeName.__name__
-        elif hasattr(builtins, self._pytypeName):
+
+        if hasattr(builtins, self._pytypeName):
             pytype = getattr(builtins, self._pytypeName)
         else:
             pytype = doImport(self._pytypeName)
@@ -308,13 +313,28 @@ def __hash__(self):
         return hash(self.name)
 
     def __repr__(self):
-        return "{}({}, pytype={}, assembler={}, components={}," \
-            " parameters={})".format(type(self).__qualname__,
-                                     self.name,
-                                     self._pytypeName,
-                                     self._assemblerClassName,
-                                     list(self.components.keys()),
-                                     self._parameters)
+        optionals = {}
+        if self._pytypeName != "object":
+            optionals["pytype"] = self._pytypeName
+        if self._assemblerClassName is not None:
+            optionals["assembler"] = self._assemblerClassName
+        if self._parameters:
+            optionals["parameters"] = self._parameters
+        if self.components:
+            optionals["components"] = self.components
+
+        # order is preserved in the dict
+        options = ", ".join(f"{k}={v!r}" for k, v in optionals.items())
+
+        # Start with mandatory fields
+        r = f"{self.__class__.__name__}({self.name!r}"
+        if options:
+            r = r + ", " + options
+        r = r + ")"
+        return r
+
+    def __str__(self):
+        return self.name
 
 
 class StorageClassFactory(metaclass=Singleton):

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

@@ -24,6 +24,7 @@
            "getObjectSize", "stripIfNotNone", "PrivateConstructorMeta",
            "NamedKeyDict", "getClassOf")
 
+import builtins
 import sys
 import functools
 from collections.abc import MutableMapping
@@ -128,10 +129,18 @@ def getFullTypeName(cls):
     -------
     name : `str`
         Full name of type.
+
+    Notes
+    -----
+    Builtins are returned without the ``builtins`` specifier included.  This
+    allows `str` to be returned as "str" rather than "builtins.str".
     """
     # If we have an instance we need to convert to a type
     if not hasattr(cls, "__qualname__"):
         cls = type(cls)
+    if hasattr(builtins, cls.__qualname__):
+        # Special case builtins such as str and dict
+        return cls.__qualname__
     return cls.__module__ + "." + cls.__qualname__
 
 

python/lsst/daf/butler/instrument.py

@@ -77,9 +77,9 @@ def getRawFormatter(self, dataId):
 
         Returns
         -------
-        formatter : `Formatter`
-            Object that reads the file into an `lsst.afw.image.Exposure`
-            instance.
+        formatter : `Formatter` class
+            Class to be used that reads the file into an
+            `lsst.afw.image.Exposure` instance.
         """
         raise NotImplementedError()
 

tests/test_formatter.py

@@ -22,11 +22,13 @@
 """Tests related to the formatter infrastructure.
 """
 
+import inspect
 import os.path
 import unittest
 
 from datasetsHelper import DatasetTestHelper
 from lsst.daf.butler import Formatter, FormatterFactory, StorageClass, DatasetType, Config, DimensionUniverse
+from lsst.daf.butler import FileDescriptor, Location
 
 TESTDIR = os.path.abspath(os.path.dirname(__file__))
 
@@ -39,24 +41,44 @@ def setUp(self):
         self.id = 0
         self.factory = FormatterFactory()
 
+        # Dummy FileDescriptor for testing getFormatter
+        self.fileDescriptor = FileDescriptor(Location("/a/b/c", "d"),
+                                             StorageClass("DummyStorageClass", dict, None))
+
+    def assertIsFormatter(self, formatter):
+        """Check that the supplied parameter is either a Formatter instance
+        or Formatter class."""
+
+        if inspect.isclass(formatter):
+            self.assertTrue(issubclass(formatter, Formatter), f"Is {formatter} a Formatter")
+        else:
+            self.assertIsInstance(formatter, Formatter)
+
     def testRegistry(self):
         """Check that formatters can be stored in the registry.
         """
         formatterTypeName = "lsst.daf.butler.formatters.fitsCatalogFormatter.FitsCatalogFormatter"
         storageClassName = "Image"
         self.factory.registerFormatter(storageClassName, formatterTypeName)
-        f = self.factory.getFormatter(storageClassName)
-        self.assertIsInstance(f, Formatter)
+        f = self.factory.getFormatter(storageClassName, self.fileDescriptor)
+        self.assertIsFormatter(f)
+        self.assertEqual(f.name(), formatterTypeName)
+        self.assertIn(formatterTypeName, str(f))
+        self.assertIn(self.fileDescriptor.location.path, str(f))
 
         fcls = self.factory.getFormatterClass(storageClassName)
-        self.assertEqual(fcls, type(f))
+        self.assertIsFormatter(fcls)
         # Defer the import so that we ensure that the infrastructure loaded
         # it on demand previously
         from lsst.daf.butler.formatters.fitsCatalogFormatter import FitsCatalogFormatter
         self.assertEqual(type(f), FitsCatalogFormatter)
 
+        with self.assertRaises(TypeError):
+            # Requires a constructor parameter
+            self.factory.getFormatter(storageClassName)
+
         with self.assertRaises(KeyError):
-            f = self.factory.getFormatter("Missing")
+            self.factory.getFormatter("Missing", self.fileDescriptor)
 
     def testRegistryWithStorageClass(self):
         """Test that the registry can be given a StorageClass object.
@@ -72,17 +94,18 @@ def testRegistryWithStorageClass(self):
         self.factory.registerFormatter(sc, formatterTypeName)
 
         # Retrieve using the class
-        f = self.factory.getFormatter(sc)
-        self.assertIsInstance(f, Formatter)
+        f = self.factory.getFormatter(sc, self.fileDescriptor)
+        self.assertIsFormatter(f)
+        self.assertEqual(f.fileDescriptor, self.fileDescriptor)
 
         # Retrieve using the DatasetType
-        f2 = self.factory.getFormatter(datasetType)
-        self.assertIsInstance(f, Formatter)
+        f2 = self.factory.getFormatter(datasetType, self.fileDescriptor)
+        self.assertIsFormatter(f2)
         self.assertEqual(f.name(), f2.name())
 
         # Class directly
         f2cls = self.factory.getFormatterClass(datasetType)
-        self.assertEqual(f2cls.name(), f.name())
+        self.assertIsFormatter(f2cls)
 
         # This might defer the import, pytest may have already loaded it
         from lsst.daf.butler.formatters.yamlFormatter import YamlFormatter
@@ -105,29 +128,29 @@ def testRegistryConfig(self):
         sc = StorageClass("DummySC", dict, None)
         refPviHsc = self.makeDatasetRef("pvi", dimensions, sc, {"instrument": "DummyHSC",
                                                                 "physical_filter": "v"})
-        refPviHscFmt = self.factory.getFormatter(refPviHsc)
-        self.assertIsInstance(refPviHscFmt, Formatter)
+        refPviHscFmt = self.factory.getFormatterClass(refPviHsc)
+        self.assertIsFormatter(refPviHscFmt)
         self.assertIn("JsonFormatter", refPviHscFmt.name())
 
         refPviNotHsc = self.makeDatasetRef("pvi", dimensions, sc, {"instrument": "DummyNotHSC",
                                                                    "physical_filter": "v"})
-        refPviNotHscFmt = self.factory.getFormatter(refPviNotHsc)
-        self.assertIsInstance(refPviNotHscFmt, Formatter)
+        refPviNotHscFmt = self.factory.getFormatterClass(refPviNotHsc)
+        self.assertIsFormatter(refPviNotHscFmt)
         self.assertIn("PickleFormatter", refPviNotHscFmt.name())
 
         # Create a DatasetRef that should fall back to using Dimensions
         refPvixHsc = self.makeDatasetRef("pvix", dimensions, sc, {"instrument": "DummyHSC",
                                                                   "physical_filter": "v"})
-        refPvixNotHscFmt = self.factory.getFormatter(refPvixHsc)
-        self.assertIsInstance(refPvixNotHscFmt, Formatter)
+        refPvixNotHscFmt = self.factory.getFormatterClass(refPvixHsc)
+        self.assertIsFormatter(refPvixNotHscFmt)
         self.assertIn("PickleFormatter", refPvixNotHscFmt.name())
 
         # Create a DatasetRef that should fall back to using StorageClass
         dimensionsNoV = universe.extract(("physical_filter", "instrument"))
         refPvixNotHscDims = self.makeDatasetRef("pvix", dimensionsNoV, sc, {"instrument": "DummyHSC",
                                                                             "physical_filter": "v"})
-        refPvixNotHscDims_fmt = self.factory.getFormatter(refPvixNotHscDims)
-        self.assertIsInstance(refPvixNotHscDims_fmt, Formatter)
+        refPvixNotHscDims_fmt = self.factory.getFormatterClass(refPvixNotHscDims)
+        self.assertIsFormatter(refPvixNotHscDims_fmt)
         self.assertIn("YamlFormatter", refPvixNotHscDims_fmt.name())
 
 

tests/test_storageClass.py

@@ -48,10 +48,17 @@ def testCreation(self):
         sc = StorageClass(className, pytype=dict)
         self.assertIsInstance(sc, StorageClass)
         self.assertEqual(sc.name, className)
+        self.assertEqual(str(sc), className)
         self.assertFalse(sc.components)
         self.assertTrue(sc.validateInstance({}))
         self.assertFalse(sc.validateInstance(""))
 
+        r = repr(sc)
+        self.assertIn("StorageClass", r)
+        self.assertIn(className, r)
+        self.assertNotIn("parameters", r)
+        self.assertIn("pytype='dict'", r)
+
         # Ensure we do not have an assembler
         with self.assertRaises(TypeError):
             sc.assembler()
@@ -63,7 +70,9 @@ def testCreation(self):
         # Include some components
         scc = StorageClass(className, pytype=PythonType, components={"comp1": sc})
         self.assertIn("comp1", scc.components)
-        self.assertIn("comp1", repr(scc))
+        r = repr(scc)
+        self.assertIn("comp1", r)
+        self.assertIn("lsst.daf.butler.core.assembler.CompositeAssembler", r)
 
         # Ensure that we have an assembler
         self.assertIsInstance(scc.assembler(), CompositeAssembler)

tests/test_utils.py

@@ -155,7 +155,7 @@ class TestButlerUtils(unittest.TestCase):
     def testTypeNames(self):
         # Check types and also an object
         tests = [(Formatter, "lsst.daf.butler.core.formatter.Formatter"),
-                 (int, "builtins.int"),
+                 (int, "int"),
                  (StorageClass, "lsst.daf.butler.core.storageClass.StorageClass"),
                  (StorageClass(None), "lsst.daf.butler.core.storageClass.StorageClass")]