Merge pull request #48 from lsst/tickets/DM-9873

DM-9873: Add nullptr_t to PropertySet/PropertyList

include/lsst/daf/base/PropertySet.h

@@ -149,6 +149,14 @@ class LSST_EXPORT PropertySet : public Citizen {
      */
     bool isPropertySetPtr(std::string const& name) const;
 
+    /**
+     * Determine if a name (possibly hierarchical) has a defined value.
+     *
+     * @param[in] name Property name to examine, possibly hierarchical.
+     * @return true if property exists and its values are undefined.
+     */
+    bool isUndefined(std::string const& name) const;
+
     /**
      * Get the number of values for a property name (possibly hierarchical).
      *

python/lsst/daf/base/propertyContainer/propertyContainerContinued.py

@@ -29,6 +29,8 @@
 import warnings
 from collections.abc import Mapping, KeysView
 
+# Ensure that C++ exceptions are properly translated to Python
+import lsst.pex.exceptions  # noqa: F401
 from lsst.utils import continueClass
 
 from .propertySet import PropertySet
@@ -143,12 +145,12 @@ def _propertyContainerElementTypeName(container, name):
     """Return name of the type of a particular element"""
     try:
         t = container.typeOf(name)
-    except (LookupError, RuntimeError) as e:
+    except LookupError as e:
         # KeyError is more commonly expected when asking for an element
         # from a mapping.
         raise KeyError(str(e))
     for checkType in ("Bool", "Short", "Int", "Long", "LongLong", "Float", "Double", "String", "DateTime",
-                      "PropertySet"):
+                      "PropertySet", "Undef"):
         if t == getattr(container, "TYPE_" + checkType):
             return checkType
     return None
@@ -270,9 +272,10 @@ def _propertyContainerSet(container, name, value, typeMenu, *args):
         return getattr(container, "set" + setType)(name, value, *args)
     # Allow for subclasses
     for checkType in typeMenu:
-        if isinstance(exemplar, checkType):
+        if (checkType is None and exemplar is None) or \
+                (checkType is not None and isinstance(exemplar, checkType)):
             return getattr(container, "set" + typeMenu[checkType])(name, value, *args)
-    raise TypeError("Unknown value type for %s: %s" % (name, t))
+    raise TypeError("Unknown value type for key '%s': %s" % (name, t))
 
 
 def _propertyContainerAdd(container, name, value, typeMenu, *args):
@@ -291,9 +294,10 @@ def _propertyContainerAdd(container, name, value, typeMenu, *args):
         return getattr(container, "add" + addType)(name, value, *args)
     # Allow for subclasses
     for checkType in typeMenu:
-        if isinstance(exemplar, checkType):
+        if (checkType is None and exemplar is None) or \
+                (checkType is not None and isinstance(exemplar, checkType)):
             return getattr(container, "add" + typeMenu[checkType])(name, value, *args)
-    raise TypeError("Unknown value type for %s: %s" % (name, t))
+    raise TypeError("Unknown value type for key '%s': %s" % (name, t))
 
 
 def _makePropertySet(state):
@@ -333,6 +337,7 @@ class PropertySet:
                  DateTime: "DateTime",
                  PropertySet: "PropertySet",
                  PropertyList: "PropertySet",
+                 None: "Undef",
                  }
 
     def get(self, name):
@@ -534,6 +539,7 @@ class PropertyList:
                  DateTime: "DateTime",
                  PropertySet: "PropertySet",
                  PropertyList: "PropertySet",
+                 None: "Undef",
                  }
 
     COMMENTSUFFIX = "#COMMENT"
@@ -690,17 +696,22 @@ def toOrderedDict(self):
 
         Returns
         -------
-        d : `~collections.OrderedDict`
+        d : `dict`
             Ordered dictionary with all properties in the order that they
             were inserted. Comments are not included.
-        """
-        from collections import OrderedDict
 
-        d = OrderedDict()
-        for name in self.getOrderedNames():
+        Notes
+        -----
+        As of Python 3.6 dicts retain their insertion order.
+        """
+        d = {}
+        for name in self:
             d[name] = _propertyContainerGet(self, name, returnStyle=ReturnStyle.AUTO)
         return d
 
+    # For PropertyList the two are equivalent
+    toDict = toOrderedDict
+
     def __eq__(self, other):
         # super() doesn't seem to work properly in @continueClass;
         # note that super with arguments seems to work at first, but actually

python/lsst/daf/base/propertyContainer/propertyList.cc

@@ -75,6 +75,7 @@ PYBIND11_MODULE(propertyList, mod) {
     declareAccessors<long long>(cls, "LongLong");
     declareAccessors<float>(cls, "Float");
     declareAccessors<double>(cls, "Double");
+    declareAccessors<nullptr_t>(cls, "Undef");
     declareAccessors<std::string>(cls, "String");
     declareAccessors<DateTime>(cls, "DateTime");
 

python/lsst/daf/base/propertyContainer/propertySet.cc

@@ -69,6 +69,7 @@ PYBIND11_MODULE(propertySet, mod) {
     cls.def("propertySetNames", &PropertySet::propertySetNames, "topLevelOnly"_a = true);
     cls.def("exists", &PropertySet::exists);
     cls.def("isArray", &PropertySet::isArray);
+    cls.def("isUndefined", &PropertySet::isUndefined);
     cls.def("isPropertySetPtr", &PropertySet::isPropertySetPtr);
     cls.def("valueCount", &PropertySet::valueCount);
     cls.def("typeOf", &PropertySet::typeOf, py::return_value_policy::reference);
@@ -91,6 +92,7 @@ PYBIND11_MODULE(propertySet, mod) {
     declareAccessors<long long>(cls, "LongLong");
     declareAccessors<float>(cls, "Float");
     declareAccessors<double>(cls, "Double");
+    declareAccessors<nullptr_t>(cls, "Undef");
     declareAccessors<std::string>(cls, "String");
     declareAccessors<DateTime>(cls, "DateTime");
     declareAccessors<std::shared_ptr<PropertySet>>(cls, "PropertySet");

python/lsst/daf/base/yaml.py

@@ -43,6 +43,10 @@
         loaderList.append(yaml.UnsafeLoader)
     except AttributeError:
         pass
+    try:
+        loaderList.append(yaml.SafeLoader)
+    except AttributeError:
+        pass
 
 
 # YAML representers for key lsst.daf.base classes

src/PropertyList.cc

@@ -282,6 +282,7 @@ INSTANTIATE(long long)
 INSTANTIATE(unsigned long long)
 INSTANTIATE(float)
 INSTANTIATE(double)
+INSTANTIATE(nullptr_t)
 INSTANTIATE(std::string)
 INSTANTIATE(Persistable::Ptr)
 INSTANTIATE(DateTime)

src/PropertySet.cc

@@ -160,6 +160,11 @@ bool PropertySet::isPropertySetPtr(std::string const& name) const {
     return i != _map.end() && i->second->back().type() == typeid(Ptr);
 }
 
+bool PropertySet::isUndefined(std::string const& name) const {
+    auto const i = _find(name);
+    return i != _map.end() && i->second->back().type() == typeid(nullptr);
+}
+
 size_t PropertySet::valueCount(std::string const& name) const {
     auto const i = _find(name);
     if (i == _map.end()) return 0;
@@ -724,6 +729,7 @@ INSTANTIATE(long long)
 INSTANTIATE(unsigned long long)
 INSTANTIATE(float)
 INSTANTIATE(double)
+INSTANTIATE(std::nullptr_t)
 INSTANTIATE(std::string)
 INSTANTIATE_PROPERTY_SET(PropertySet::Ptr)
 INSTANTIATE(Persistable::Ptr)

tests/test_PropertyList.py

@@ -58,7 +58,10 @@ def testScalar(self):
         apl.set("int2", 2009)
         apl.set("dt", dafBase.DateTime("20090402T072639.314159265Z", dafBase.DateTime.UTC))
         apl.set("subclass", FloatSubClass(1.23456789))
+        apl.set("undef", None)
 
+        self.assertTrue(apl.isUndefined("undef"))
+        self.assertFalse(apl.isUndefined("string"))
         self.assertEqual(apl.typeOf("bool"), dafBase.PropertyList.TYPE_Bool)
         self.assertEqual(apl.getBool("bool"), True)
         self.assertEqual(apl.typeOf("short"), dafBase.PropertyList.TYPE_Short)
@@ -85,8 +88,19 @@ def testScalar(self):
         self.assertEqual(apl.typeOf("dt"), dafBase.PropertyList.TYPE_DateTime)
         self.assertEqual(apl.getDateTime("dt").nsecs(), 1238657233314159265)
         self.assertEqual(apl.getDouble("subclass"), 1.23456789)
+
+        self.assertIsNone(apl.getScalar("undef"))
+        self.assertEqual(apl.typeOf("undef"), dafBase.PropertyList.TYPE_Undef)
+        with self.assertWarns(DeprecationWarning):
+            self.assertIsNone(apl.get("undef"))
         self.checkPickle(apl)
 
+        # Now replace the undef value with a defined value
+        apl.set("undef", "not undefined")
+        self.assertEqual(apl.getScalar("undef"), "not undefined")
+        self.assertFalse(apl.isUndefined("undef"))
+        self.assertEqual(apl.typeOf("undef"), dafBase.PropertyList.TYPE_String)
+
     def testGetDefault(self):
         apl = dafBase.PropertyList()
         apl.setInt("int", 42)
@@ -176,6 +190,7 @@ def testGetScalarThrow(self):
         apl.setFloat("float", 3.14159)
         apl.setDouble("double", 2.718281828459045)
         apl.setString("string", "bar")
+
         with self.assertWarns(DeprecationWarning):
             with self.assertRaises(KeyError):
                 apl.get("foo")
@@ -271,6 +286,7 @@ def testToOrderedDict(self):
         apl.set("NAXIS", 2)
         apl.set("RA", 3.14159)
         apl.set("DEC", 2.71828)
+        apl.set("FILTER", None)
         apl.set("COMMENT", "This is a test")
         apl.add("COMMENT", "This is a test line 2")
         correct = OrderedDict([
@@ -279,6 +295,7 @@ def testToOrderedDict(self):
             ("NAXIS", 2),
             ("RA", 3.14159),
             ("DEC", 2.71828),
+            ("FILTER", None),
             ("COMMENT", ["This is a test", "This is a test line 2"])
         ])
         self.assertEqual(apl.toOrderedDict(), correct)
@@ -516,7 +533,6 @@ def testPropertySetNames(self):
 
         # There are no PropertySets inside flattened PropertyList
         v = set(apl.propertySetNames())
-        print(v)
         self.assertEqual(len(v), 0)
 
     def testGetAs(self):

tests/test_PropertySet_1.cc

@@ -73,6 +73,7 @@ BOOST_AUTO_TEST_CASE(getScalar) { /* parasoft-suppress LsstDm-3-1 LsstDm-3-4a Ls
     ps.set<std::string>("char*", "foo");
     ps.set("char*2", "foo2");
     ps.set("string", std::string("bar"));
+    ps.set("undef", nullptr);
 
     BOOST_CHECK_EQUAL(ps.get<bool>("bool"), true);
     BOOST_CHECK_EQUAL(ps.get<char>("char"), '*');
@@ -84,6 +85,9 @@ BOOST_AUTO_TEST_CASE(getScalar) { /* parasoft-suppress LsstDm-3-1 LsstDm-3-4a Ls
     BOOST_CHECK_EQUAL(ps.get<std::string>("char*"), "foo");
     BOOST_CHECK_EQUAL(ps.get<std::string>("char*2"), "foo2");
     BOOST_CHECK_EQUAL(ps.get<std::string>("string"), "bar");
+    BOOST_CHECK_EQUAL(ps.get<nullptr_t>("undef"), nullptr);
+    BOOST_CHECK_EQUAL(ps.isUndefined("string"), false);
+    BOOST_CHECK_EQUAL(ps.isUndefined("undef"), true);
 }
 
 BOOST_AUTO_TEST_CASE(resetScalar) { /* parasoft-suppress LsstDm-3-1 LsstDm-3-4a LsstDm-5-25 LsstDm-4-6 "Boost

tests/test_PropertySet_2.py

@@ -57,6 +57,7 @@ def testScalar(self):
         ps.set("int2", 2009)
         ps.set("dt", dafBase.DateTime("20090402T072639.314159265Z", dafBase.DateTime.UTC))
         ps.set("blank", "")
+        ps.set("undef", None)
 
         self.assertEqual(ps.typeOf("bool"), dafBase.PropertySet.TYPE_Bool)
         self.assertEqual(ps.getBool("bool"), True)
@@ -84,8 +85,20 @@ def testScalar(self):
         self.assertEqual(ps.typeOf("dt"), dafBase.PropertySet.TYPE_DateTime)
         self.assertEqual(ps.getDateTime("dt").nsecs(), 1238657233314159265)
         self.assertEqual(ps.getString("blank"), "")
+
+        self.assertIsNone(ps.getScalar("undef"))
+        self.assertEqual(ps.typeOf("undef"), dafBase.PropertyList.TYPE_Undef)
+        with self.assertWarns(DeprecationWarning):
+            self.assertIsNone(ps.get("undef"))
+
         self.checkPickle(ps)
 
+        # Now replace the undef value with a defined value
+        ps.set("undef", "not undefined")
+        self.assertEqual(ps.getScalar("undef"), "not undefined")
+        self.assertFalse(ps.isUndefined("undef"))
+        self.assertEqual(ps.typeOf("undef"), dafBase.PropertyList.TYPE_String)
+
     def testNumPyScalars(self):
         """Test that we can also pass NumPy array scalars to PropertySet setters.
         """
@@ -526,6 +539,7 @@ def testToDict(self):
         ps2.set("char*", "foo")
         ps2.setString("string", "bar")
         ps2.set("dt", dafBase.DateTime("20090402T072639.314159265Z", dafBase.DateTime.UTC))
+        ps2.set("undef", None)
 
         d = ps2.toDict()
         self.assertIsInstance(d, dict)
@@ -534,6 +548,7 @@ def testToDict(self):
         self.assertAlmostEqual(d["float"], 3.14159, 6)
         self.assertIsInstance(d["double"], float)
         self.assertEqual(d["double"], 2.718281828459045)
+        self.assertIsNone(d["undef"])
 
         self.assertIsInstance(d["char*"], str)
         self.assertEqual(d["char*"], "foo")

tests/test_dict.py

@@ -42,6 +42,7 @@ def setUp(self):
         pl.setString("string", "bar")
         pl.set("int2", 2009, "int comment")
         pl.set("dt", lsst.daf.base.DateTime("20090402T072639.314159265Z", lsst.daf.base.DateTime.UTC))
+        pl.set("undef", None)
 
         ps = lsst.daf.base.PropertySet()
         ps.setBool("bool", True)
@@ -59,6 +60,7 @@ def setUp(self):
         ps.set("dt", lsst.daf.base.DateTime("20090402T072639.314159265Z", lsst.daf.base.DateTime.UTC))
         ps.set("blank", "")
         ps.addInt("int", 2009)
+        ps.set("undef", None)
 
         ps2 = lsst.daf.base.PropertySet()
         ps2.setBool("bool2", False)
@@ -108,6 +110,7 @@ def testDictPropertySet(self):
         # Compare dict-like interface to pure dict version
         d = container.toDict()
         self.assertEqual(len(d), len(container))
+        self.assertIsNone(d["undef"])
 
         # Set some values
         container["new"] = "string"
@@ -116,7 +119,7 @@ def testDictPropertySet(self):
         self.assertIn("dot", container)
 
         keys = container.keys()
-        self.assertEqual(len(keys), 16)
+        self.assertEqual(len(keys), 17)
         for k in keys:
             self.assertIn(k, container)
 
@@ -127,6 +130,9 @@ def testDictPropertySet(self):
         ps2["newint"] = 5
         self.assertEqual(container.getScalar("newps2.newint"), ps2.getScalar("newint"))
 
+        ps2["undef2"] = None
+        self.assertIn("undef2", ps2)
+
         # Dict should be converted to a PropertySet
         container["dict"] = {"a": 1, "b": 2}
         self.assertEqual(container.getScalar("dict.b"), 2)
@@ -145,14 +151,15 @@ def testDictPropertyList(self):
         # Compare dict-like interface to pure dict version
         d = container.toDict()
         self.assertEqual(len(d), len(container))
+        self.assertIsNone(d["undef"])
 
         # Set some values
         container["new"] = "string"
         container["array"] = [1, 2, 3]
         container["dot.delimited"] = "delimited"
 
         keys = container.keys()
-        self.assertEqual(len(keys), 16)
+        self.assertEqual(len(keys), 17)
         for k in keys:
             self.assertIn(k, container)
 
@@ -165,6 +172,9 @@ def testDictPropertyList(self):
         self.assertNotIn("newps2.newinst", container)
         self.assertIn("newint", ps2)
 
+        ps2["undef2"] = None
+        self.assertIn("undef2", ps2)
+
         # Dict should be converted to a PropertySet
         container["dict"] = {"a": 1, "b": 2}
         self.assertEqual(container.getScalar("dict.b"), 2)