DM-34326 Collect imports in ConfigDictField

.github/workflows/build.yaml

@@ -1,8 +1,10 @@
 name: build_and_test
 
 on:
-  - push
-  - pull_request
+  push:
+    branches:
+      - main
+  pull_request:
 
 jobs:
   build_and_test:
@@ -16,7 +18,7 @@ jobs:
       - name: Set up Python
         uses: actions/setup-python@v2
         with:
-          python-version: 3.7
+          python-version: 3.8
 
       - name: Install yaml
         run: sudo apt-get install libyaml-dev

.github/workflows/formatting.yaml

@@ -21,7 +21,7 @@ jobs:
         run: pip install isort black
 
       - name: Run isort
-        run: isort --check-only python/ tests/
+        run: isort --check-only --diff python/ tests/
 
       - name: Run black
         run: black --check --verbose --diff python/ tests/

.github/workflows/lint.yaml

@@ -1,8 +1,10 @@
 name: lint
 
 on:
-  - push
-  - pull_request
+  push:
+    branches:
+      - main
+  pull_request:
 
 jobs:
   lint:
@@ -13,7 +15,7 @@ jobs:
       - name: Set up Python
         uses: actions/setup-python@v2
         with:
-          python-version: 3.7
+          python-version: 3.8
 
       - name: Install
         run: pip install -r <(curl https://raw.githubusercontent.com/lsst/linting/main/requirements.txt)

.pre-commit-config.yaml

@@ -6,7 +6,7 @@ repos:
       - id: end-of-file-fixer
       - id: trailing-whitespace
   - repo: https://github.com/psf/black
-    rev: 21.11b1
+    rev: 22.3.0
     hooks:
       - id: black
         # It is recommended to specify the latest version of Python

python/lsst/pex/config/configChoiceField.py

@@ -33,7 +33,7 @@
 
 from .callStack import getCallStack, getStackFrame
 from .comparison import compareConfigs, compareScalars, getComparisonName
-from .config import Config, Field, FieldValidationError, _joinNamePath, _typeStr, UnexpectedProxyUsageError
+from .config import Config, Field, FieldValidationError, UnexpectedProxyUsageError, _joinNamePath, _typeStr
 
 
 class SelectionSet(collections.abc.MutableSet):

python/lsst/pex/config/configDictField.py

@@ -213,6 +213,14 @@ def toDict(self, instance):
 
         return dict_
 
+    def _collectImports(self, instance, imports):
+        # docstring inherited from Field
+        configDict = self.__get__(instance)
+        if configDict is not None:
+            for v in configDict.values():
+                v._collectImports()
+                imports |= v._imports
+
     def save(self, outfile, instance):
         configDict = self.__get__(instance)
         fullname = _joinNamePath(instance._name, self.name)

python/lsst/pex/config/configurableField.py

@@ -32,7 +32,7 @@
 
 from .callStack import getCallStack, getStackFrame
 from .comparison import compareConfigs, getComparisonName
-from .config import Config, Field, FieldValidationError, _joinNamePath, _typeStr, UnexpectedProxyUsageError
+from .config import Config, Field, FieldValidationError, UnexpectedProxyUsageError, _joinNamePath, _typeStr
 
 
 class ConfigurableInstance:

python/lsst/pex/config/dictField.py

@@ -36,10 +36,10 @@
     Config,
     Field,
     FieldValidationError,
+    UnexpectedProxyUsageError,
     _autocast,
     _joinNamePath,
     _typeStr,
-    UnexpectedProxyUsageError,
 )
 
 

python/lsst/pex/config/history.py

@@ -202,17 +202,13 @@ def format(config, name=None, writeSourceLine=True, prefix="", verbose=False):
     for value, stack, label in config.history.get(name, []):
         output = []
         for frame in stack:
-            if (
-                frame.function
-                in (
-                    "__new__",
-                    "__set__",
-                    "__setattr__",
-                    "execfile",
-                    "wrapper",
-                )
-                or os.path.split(frame.filename)[1] in ("argparse.py", "argumentParser.py")
-            ):
+            if frame.function in (
+                "__new__",
+                "__set__",
+                "__setattr__",
+                "execfile",
+                "wrapper",
+            ) or os.path.split(frame.filename)[1] in ("argparse.py", "argumentParser.py"):
                 if not verbose:
                     continue
 

setup.cfg

@@ -34,7 +34,7 @@ where=python
 [flake8]
 max-line-length = 110
 max-doc-length = 79
-ignore = E133, E226, E228, N802, N803, N806, N812, N815, N816, W503, E203
+ignore = W503, E203, E228, N802, N803, N806, N812, N815, N816
 exclude =
   bin,
   doc,
@@ -47,4 +47,4 @@ exclude =
 
 [tool:pytest]
 addopts = --flake8
-flake8-ignore = E133 E226 E228 N802 N803 N806 N812 N815 N816 W503 E203
+flake8-ignore = W503 E203 N802 N803 N806 N812 N815 N816

tests/test_configChoiceField.py

@@ -149,8 +149,7 @@ def testNoneValue(self):
         self.config.validate()
 
     def testNoPickle(self):
-        """Test that pickle support is disabled for the proxy container.
-        """
+        """Test that pickle support is disabled for the proxy container."""
         with self.assertRaises(pexConfig.UnexpectedProxyUsageError):
             pickle.dumps(self.config.c)
         with self.assertRaises(pexConfig.UnexpectedProxyUsageError):

tests/test_configDictField.py

@@ -34,6 +34,10 @@
 class Config1(pexConfig.Config):
     f = pexConfig.Field("f", float, default=3.0)
 
+    def _collectImports(self):
+        # Exists to test that imports of dict values are collected
+        self._imports.add("builtins")
+
 
 class Config2(pexConfig.Config):
     d1 = pexConfig.ConfigDictField("d1", keytype=str, itemtype=Config1, itemCheck=lambda x: x.f > 0)
@@ -114,6 +118,10 @@ def testSave(self):
         c = Config2(d1={"a": Config1(f=4)})
         c.save("configDictTest.py")
 
+        # verify _collectImports is called on all the configDictValues
+        stringOutput = c.saveToString()
+        self.assertIn("import builtins", stringOutput)
+
         rt = Config2()
         rt.load("configDictTest.py")
 

tests/test_configurableField.py

@@ -142,8 +142,7 @@ def testPersistence(self):
         self.assertEqual(c.c2.target, r.c2.target)
 
     def testNoPickle(self):
-        """Test that pickle support is disabled for the proxy container.
-        """
+        """Test that pickle support is disabled for the proxy container."""
         c = Config2()
         with self.assertRaises(pexConf.UnexpectedProxyUsageError):
             pickle.dumps(c.c2)

tests/test_dictField.py

@@ -144,8 +144,7 @@ def testEquality(self):
         self.assertTrue(pexConfig.compareConfigs("test", c1, c2))
 
     def testNoPickle(self):
-        """Test that pickle support is disabled for the proxy container.
-        """
+        """Test that pickle support is disabled for the proxy container."""
         c = Config1()
         with self.assertRaises(pexConfig.UnexpectedProxyUsageError):
             pickle.dumps(c.d4)

tests/test_listField.py

@@ -163,8 +163,7 @@ def testNoArbitraryAttributes(self):
         self.assertRaises(pexConfig.FieldValidationError, setattr, c.l1, "should", "fail")
 
     def testNoPickle(self):
-        """Test that pickle support is disabled for the proxy container.
-        """
+        """Test that pickle support is disabled for the proxy container."""
         c = Config2()
         with self.assertRaises(pexConfig.UnexpectedProxyUsageError):
             pickle.dumps(c.ls)