From f4be91c18b05ec3a8fcbfb4a71fa601898da2539 Mon Sep 17 00:00:00 2001
From: Tim Jenness <tjenness@lsst.org>
Date: Tue, 1 Feb 2022 15:43:28 -0700
Subject: [PATCH 1/2] Use type hinting when converting to PropertySet

There can be a large slowdown in PropertySet creation if
PropertySet.set() is called without a type being specified.
The Configs know the types of their fields so use those
types explicitly.

For integer types always use LongLong.
---
 python/lsst/pex/config/convert.py | 60 ++++++++++++++++++++++++++++---
 tests/test_Config.py              | 11 ++++++
 2 files changed, 67 insertions(+), 4 deletions(-)

diff --git a/python/lsst/pex/config/convert.py b/python/lsst/pex/config/convert.py
index 0baff1c9..04ae4019 100644
--- a/python/lsst/pex/config/convert.py
+++ b/python/lsst/pex/config/convert.py
@@ -33,13 +33,65 @@
     dafBase = None
 
 
-def _helper(ps, prefix, dict_):
+def _helper(config, ps, prefix, dict_, ext_dtype=None):
+    """Convert to PropertySet using types from the Config."""
+    try:
+        fields = config._fields
+    except AttributeError:
+        # dictField.Dict will not have _fields attribute.
+        fields = {}
+
     for k, v in dict_.items():
         name = prefix + "." + k if prefix is not None else k
         if isinstance(v, dict):
-            _helper(ps, name, v)
+            field = fields.get(k, None)
+
+            if hasattr(field, "itemtype"):
+                itemtype = field.itemtype
+            else:
+                itemtype = None
+
+            if hasattr(field, "typemap"):
+                # ChoiceField needs special layout.
+                # name: Default choice
+                # values: {ChoiceA: {}, ChoiceB: {}}
+                config_choice = getattr(config, k)
+                # Follow the toDict code for ConfigChoiceField
+                if hasattr(config_choice, "multi") and config_choice.multi:
+                    ps[f"{name}.names"] = config_choice.names
+                else:
+                    ps[f"{name}.name"] = config_choice.name
+                for k, v in config_choice.items():
+                    sub_name = f"{name}.values.{k}"
+                    _helper(config_choice[k], ps, sub_name, v.toDict())
+                continue
+
+            # Pass in the item type if we have it.
+            _helper(getattr(config, k), ps, name, v, ext_dtype=itemtype)
         elif v is not None:
-            ps.set(name, v)
+            try:
+                field = fields[k]
+            except KeyError:
+                dtype = ext_dtype
+                field = None
+            else:
+                dtype = field.dtype
+
+            # DictField and ListField have a dtype so ask for it.
+            if hasattr(field, "itemtype"):
+                dtype = field.itemtype
+
+            if dtype is str:
+                ps.setString(name, v)
+            elif dtype is bool:
+                ps.setBool(name, v)
+            elif dtype is int:
+                ps.setLongLong(name, v)
+            elif dtype is float:
+                ps.setDouble(name, v)
+            else:
+                # Fall back should not happen.
+                ps.set(name, v)
 
 
 def makePropertySet(config):
@@ -66,7 +118,7 @@ def makePropertySet(config):
 
     if config is not None:
         ps = dafBase.PropertySet()
-        _helper(ps, None, config.toDict())
+        _helper(config, ps, None, config.toDict())
         return ps
     else:
         return None
diff --git a/tests/test_Config.py b/tests/test_Config.py
index ad9d332d..1bc7679b 100644
--- a/tests/test_Config.py
+++ b/tests/test_Config.py
@@ -447,6 +447,10 @@ class III(AAA):
 
     @unittest.skipIf(dafBase is None, "lsst.daf.base is required")
     def testConvertPropertySet(self):
+        # assertEqual for hierarchical dicts does not seem to sort keys
+        # so instead convert to JSON strings before comparing.
+        import json
+
         ps = pexConfig.makePropertySet(self.simple)
         self.assertFalse(ps.exists("i"))
         self.assertEqual(ps.getScalar("f"), self.simple.f)
@@ -457,6 +461,13 @@ def testConvertPropertySet(self):
         ps = pexConfig.makePropertySet(self.comp)
         self.assertEqual(ps.getScalar("c.f"), self.comp.c.f)
 
+        # Check property set matches the dict.
+        # Have to remove the None item from the dict.
+        config_dict = self.comp.toDict()
+        for k in ("p", "r"):
+            del config_dict[k]["values"]["AAA"]["i"]
+        self.assertEqual(json.dumps(ps.toDict(), sort_keys=True), json.dumps(config_dict, sort_keys=True))
+
     def testFreeze(self):
         self.comp.freeze()
 

From e72110fabb89799013cf5608bd8071f38d2a2eda Mon Sep 17 00:00:00 2001
From: Tim Jenness <tjenness@lsst.org>
Date: Tue, 1 Feb 2022 16:39:03 -0700
Subject: [PATCH 2/2] For integers we can not force to LongLong

---
 python/lsst/pex/config/convert.py | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/python/lsst/pex/config/convert.py b/python/lsst/pex/config/convert.py
index 04ae4019..9e74f145 100644
--- a/python/lsst/pex/config/convert.py
+++ b/python/lsst/pex/config/convert.py
@@ -86,7 +86,13 @@ def _helper(config, ps, prefix, dict_, ext_dtype=None):
             elif dtype is bool:
                 ps.setBool(name, v)
             elif dtype is int:
-                ps.setLongLong(name, v)
+                # User code requires that small numbers are stored in
+                # small integers and LongLong be reserved for larger
+                # ints. This means that we have to either use an
+                # optimized version of the logic from PropertySet
+                # (we know it does not exist and that it is an integer type)
+                # or just use the generic setter.
+                ps.set(name, v)
             elif dtype is float:
                 ps.setDouble(name, v)
             else: