code review fixes

lsst-dm · Nov 17, 2017 · 9971953 · 9971953
1 parent 617ff0e
commit 9971953
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 36 deletions.
diff --git a/python/lsst/daf/persistence/butler.py b/python/lsst/daf/persistence/butler.py
@@ -1601,18 +1601,6 @@ def cfgNeedsFixup(self, cfg):
                     cfg._parents[i] = cfg._normalizeParents(cfg.root, [parentCfg])[0]
 
 
-def checkCfgs(cfgList):
-    butler = Butler()
-    fixupCfgs = []
-    okCfgs = []
-    for cfgPath in cfgList:
-        cfg = PosixStorage.getRepositoryCfg(cfgPath)
-        if butler.cfgNeedsFixup(cfg):
-            fixupCfgs.append(cfgPath)
-        else:
-            okCfgs.append(cfgPath)
-
-
 def _unreduce(initArgs, datasetTypeAliasDict):
     mapperArgs = initArgs.pop('mapperArgs')
     initArgs.update(mapperArgs)

diff --git a/tests/test_DM-12117.py b/tests/test_DM-12117.py
@@ -48,7 +48,7 @@ class TestDM12117(unittest.TestCase):
     """Test case for basic Butler operations."""
 
     def setUp(self):
-        self.testDir = tempfile.mkdtemp(dir=ROOT, prefix='test_DM-12117-')
+            self.testDir = tempfile.mkdtemp(dir=ROOT, prefix='test_DM-12117-')
 
     def tearDown(self):
         if os.path.exists(self.testDir):
@@ -65,36 +65,61 @@ def repoBYaml(mapperArgs):
 dirty: true
 """.format(mapperArgs)
 
-    def test(self):
-        """Test that an Old Butler parent repo that is can be loaded by a New Butler output repo and that the
-        output repo's mapper args are used by the OldButler repo.
+    def testOldButlerParentWithoutMapperArgs(self):
+        """Test that an Old Butler parent repo that is can be loaded by a New
+        Butler output repo and that the output repo's mapper args are used by
+        the OldButler repo.
 
         1. create an Old Butler repo
         2. create a New Butler repo without mapper args
         3. reload that New Butler repo without naming its parent as an input
         4. verify that the parent is loaded as an input
 
-        5. do the same, but specify a mapper arg, and verify that that mapper arg is passed to the parent as
-           well as the root repo.
+        5. do the same, but specify a mapper arg, and verify that that mapper
+           arg is passed to the parent as well as the root repo.
         """
-        for mapperArgs in ({}, {'calib': 'foo'}):
-            repoAPath = os.path.join(self.testDir, 'repoA')
-            repoBPath = os.path.join(self.testDir, 'repoB')
-            os.makedirs(repoAPath)
-            with open(os.path.join(repoAPath, '_mapper'), 'w') as f:
-                f.write('lsst.daf.persistence.test.EmptyTestMapper')
-            os.makedirs(repoBPath)
-            with open(os.path.join(repoBPath, 'repositoryCfg.yaml'), 'w') as f:
-                f.write(self.repoBYaml(mapperArgs))
-            butler = dp.Butler(repoBPath)
-            self.assertEqual(butler._repos.inputs()[0].repo._mapper.root, repoBPath)
-            self.assertEqual(butler._repos.inputs()[1].repo._mapper.root, repoAPath)
-            self.assertEqual(butler._repos.outputs()[0].repo._mapper.root, repoBPath)
-            self.assertEqual(butler._repos.inputs()[0].repo._mapper.kwargs.get('calib'),
-                             mapperArgs.get('calib'))
-            self.assertEqual(butler._repos.inputs()[1].repo._mapper.kwargs.get('calib'),
-                             mapperArgs.get('calib'))
-            self.tearDown()
+        repoAPath = os.path.join(self.testDir, 'repoA')
+        repoBPath = os.path.join(self.testDir, 'repoB')
+        os.makedirs(repoAPath)
+        with open(os.path.join(repoAPath, '_mapper'), 'w') as f:
+            f.write('lsst.daf.persistence.test.EmptyTestMapper')
+        os.makedirs(repoBPath)
+        with open(os.path.join(repoBPath, 'repositoryCfg.yaml'), 'w') as f:
+            f.write(self.repoBYaml({}))
+        butler = dp.Butler(repoBPath)
+        self.assertEqual(butler._repos.inputs()[0].repo._mapper.root, repoBPath)
+        self.assertEqual(butler._repos.inputs()[1].repo._mapper.root, repoAPath)
+        self.assertEqual(butler._repos.outputs()[0].repo._mapper.root, repoBPath)
+        self.assertEqual(butler._repos.inputs()[0].repo._mapper.kwargs.get('calib'), None)
+        self.assertEqual(butler._repos.inputs()[1].repo._mapper.kwargs.get('calib'), None)
+
+
+    def testOldButlerParentWithMapperArgs(self):
+        """Test that an Old Butler parent repo that is can be loaded by a New
+        Butler output repo and that the output repo's mapper args are used by
+        the OldButler repo.
+
+        1. create an Old Butler repo
+        2. create a New Butler repo with a mapper argument, and verify that
+           that mapper arg is passed to the parent as well as the root repo.
+        3. reload that New Butler repo without naming its parent as an input
+        4. verify that the parent is loaded as an input
+        """
+        repoAPath = os.path.join(self.testDir, 'repoA')
+        repoBPath = os.path.join(self.testDir, 'repoB')
+        os.makedirs(repoAPath)
+        with open(os.path.join(repoAPath, '_mapper'), 'w') as f:
+            f.write('lsst.daf.persistence.test.EmptyTestMapper')
+        os.makedirs(repoBPath)
+        mapperArgs = {'calib': 'foo'}
+        with open(os.path.join(repoBPath, 'repositoryCfg.yaml'), 'w') as f:
+            f.write(self.repoBYaml(mapperArgs))
+        butler = dp.Butler(repoBPath)
+        self.assertEqual(butler._repos.inputs()[0].repo._mapper.root, repoBPath)
+        self.assertEqual(butler._repos.inputs()[1].repo._mapper.root, repoAPath)
+        self.assertEqual(butler._repos.outputs()[0].repo._mapper.root, repoBPath)
+        self.assertEqual(butler._repos.inputs()[0].repo._mapper.kwargs.get('calib'), mapperArgs.get('calib'))
+        self.assertEqual(butler._repos.inputs()[1].repo._mapper.kwargs.get('calib'), mapperArgs.get('calib'))
 
 
 class MemoryTester(lsst.utils.tests.MemoryTestCase):