Implement review comments

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

@@ -32,9 +32,27 @@ class RegistryConfig(Config):
 
 class Registry(metaclass=ABCMeta):
     """Registry interface.
+
+    Parameters
+    ----------
+    config : `RegistryConfig`
     """
     @staticmethod
     def fromConfig(config):
+        """Create `Registry` subclass instance from `config`.
+
+        Uses ``registry.cls`` from `config` to determine which subclass to instantiate.
+
+        Parameters
+        ----------
+        config : `RegistryConfig`, `Config` or `str`
+            Registry configuration
+
+        Returns
+        -------
+        registry : `Registry` (subclass)
+            A new `Registry` subclass instance.
+        """
         if not isinstance(config, RegistryConfig):
             if isinstance(config, str):
                 config = Config(config)
@@ -46,12 +64,6 @@ def fromConfig(config):
         return cls(config=config)
 
     def __init__(self, config):
-        """Constructor
-
-        Parameters
-        ----------
-        config : `RegistryConfig`
-        """
         assert isinstance(config, RegistryConfig)
         self.config = config
 

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

@@ -19,30 +19,14 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+from .utils import iterable
 from .config import Config
 from sqlalchemy import Column, String, Integer, Boolean, LargeBinary, DateTime,\
     Float, ForeignKey, ForeignKeyConstraint, Table, MetaData
 
 metadata = None  # Needed to make disabled test_hsc not fail on import
 
 
-def iterable(a):
-    """Make input iterable.
-
-    There are three cases, when the input is:
-    - iterable, but not a string -> iterate over elements (e.g. [i for i in a])
-    - a string -> return single element iterable (e.g. [a])
-    - not iterable -> return single elment iterable (e.g. [a]).
-    """
-    if hasattr(a, 'encode'):
-        yield a
-        return
-    try:
-        yield from a
-    except:
-        yield a
-
-
 class SchemaConfig(Config):
     """Schema configuration.
     """
@@ -62,23 +46,21 @@ def tables(self):
 
 class Schema:
     """The SQL schema for a Butler Registry.
+
+    Parameters
+    ----------
+    config : `SchemaConfig` or `str`
+        Load configuration
+
+    Attributes
+    ----------
+    metadata : `sqlalchemy.MetaData`
+        The sqlalchemy schema description
     """
     VALID_COLUMN_TYPES = {'string': String, 'int': Integer, 'float': Float,
                           'bool': Boolean, 'blob': LargeBinary, 'datetime': DateTime}
 
     def __init__(self, config):
-        """Constructor
-
-        Parameters
-        ----------
-        config : `SchemaConfig` or `str`
-            Load configuration
-        
-        Attributes
-        ----------
-        metadata : `sqlalchemy.MetaData`
-            The sqlalchemy schema description
-        """
         self.config = SchemaConfig(config)
         self.metadata = MetaData()
         for tableName, tableDescription in self.config.tables.items():
@@ -127,10 +109,10 @@ def makeColumn(self, columnDescription):
         -------
         c : `sqlalchemy.Column`
             The created `Column` entry.
-        
+
         Raises
         ------
-        e : `ValueError
+        ValueError
             If the column description contains unsupported arguments
         """
         description = columnDescription.copy()

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

@@ -20,6 +20,23 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 
+def iterable(a):
+    """Make input iterable.
+
+    There are three cases, when the input is:
+    - iterable, but not a string -> iterate over elements (e.g. [i for i in a])
+    - a string -> return single element iterable (e.g. [a])
+    - not iterable -> return single elment iterable (e.g. [a]).
+    """
+    if isinstance(a, str):
+        yield a
+        return
+    try:
+        yield from a
+    except Exception:
+        yield a
+
+
 def allSlots(self):
     """
     Return combined __slots__ for all classes in objects mro.

python/lsst/daf/butler/registries/sqlRegistry.py

@@ -31,15 +31,13 @@ class SqlRegistryConfig(RegistryConfig):
 
 class SqlRegistry(Registry):
     """Registry backed by a SQL database.
+
+    Parameters
+    ----------
+    config : `SqlRegistryConfig` or `str`
+        Load configuration
     """
     def __init__(self, config):
-        """Constructor
-
-        Parameters
-        ----------
-        config : `SqlRegistryConfig` or `str`
-            Load configuration
-        """
         super().__init__(config)
 
         self.config = SqlRegistryConfig(config)
@@ -105,7 +103,7 @@ def addDataset(self, ref, uri, components, run, producer=None):
 
         Raises
         ------
-        e: `Exception`
+        Exception
             If a `Dataset` with the given `DatasetRef` already exists in the
             given Collection.
         """
@@ -223,7 +221,7 @@ def markInputUsed(self, quantum, ref):
 
         Raises
         ------
-        e: `Exception`
+        Exception
             If `ref` is not already in the predicted inputs list.
         """
         raise NotImplementedError("Must be implemented by subclass")

tests/test_hsc.py

@@ -24,6 +24,7 @@
 import lsst.daf.butler.ci_hsc
 
 
+@unittest.skip("Does not use up-to-date schema")
 class TestDataTestCase(unittest.TestCase):
     """A test case for the ci_hsc Registry test data."""
 

tests/test_schema.py

@@ -1,10 +1,10 @@
+# This file is part of daf_butler.
 #
-# LSST Data Management System
-#
-# Copyright 2008-2018  AURA/LSST.
-#
-# This product includes software developed by the
-# LSST Project (http://www.lsst.org/).
+# 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
@@ -16,10 +16,8 @@
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 # GNU General Public License for more details.
 #
-# You should have received a copy of the LSST License Statement and
-# the GNU General Public License along with this program.  If not,
-# see <https://www.lsstcorp.org/LegalNotices/>.
-#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import os
 import unittest
@@ -28,24 +26,12 @@
 
 import lsst.utils.tests
 
-from lsst.daf.butler.core.schema import SchemaConfig, Schema, Table, Column, iterable
+from lsst.daf.butler.core.utils import iterable
+from lsst.daf.butler.core.schema import SchemaConfig, Schema, Table, Column
 
 """Tests for Schema.
 """
 
-class IterableTestCase(lsst.utils.tests.TestCase):
-    """Tests for `iterable` helper.
-    """
-    def testNonIterable(self):
-        self.assertEqual(list(iterable(0)), [0, ])
-
-    def testString(self):
-        self.assertEqual(list(iterable("hello")), ["hello", ])
-
-    def testIterableNoString(self):
-        self.assertEqual(list(iterable([0, 1, 2])), [0, 1, 2])
-        self.assertEqual(list(iterable(["hello", "world"])), ["hello", "world"])
-
 
 class SchemaTestCase(lsst.utils.tests.TestCase):
     """Tests for Schema.

tests/test_sqlRegistry.py

@@ -1,10 +1,10 @@
+# This file is part of daf_butler.
 #
-# LSST Data Management System
-#
-# Copyright 2008-2018  AURA/LSST.
-#
-# This product includes software developed by the
-# LSST Project (http://www.lsst.org/).
+# 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
@@ -16,10 +16,8 @@
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 # GNU General Public License for more details.
 #
-# You should have received a copy of the LSST License Statement and
-# the GNU General Public License along with this program.  If not,
-# see <https://www.lsstcorp.org/LegalNotices/>.
-#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import os
 import unittest

tests/test_utils.py

@@ -22,10 +22,26 @@
 import unittest
 import inspect
 
-from lsst.daf.butler.core.utils import doImport
+import lsst.utils.tests
+
+from lsst.daf.butler.core.utils import iterable, doImport
 from lsst.daf.butler.core.formatter import Formatter
 
 
+class IterableTestCase(lsst.utils.tests.TestCase):
+    """Tests for `iterable` helper.
+    """
+    def testNonIterable(self):
+        self.assertEqual(list(iterable(0)), [0, ])
+
+    def testString(self):
+        self.assertEqual(list(iterable("hello")), ["hello", ])
+
+    def testIterableNoString(self):
+        self.assertEqual(list(iterable([0, 1, 2])), [0, 1, 2])
+        self.assertEqual(list(iterable(["hello", "world"])), ["hello", "world"])
+
+
 class ImportTestCase(unittest.TestCase):
     """Basic tests of doImport."""