Merge pull request #88 from PDAL/issue-87

Use out_metadata for output metadata

Author: abellgithub

pdal/plang/Environment.cpp

@@ -79,6 +79,25 @@ static void loadPython()
 }
 #endif
 
+namespace
+{
+
+std::string readPythonString(PyObject* dict, const std::string& key)
+{
+    std::string s;
+
+    PyObject* o = PyDict_GetItemString(dict, key.c_str());
+    if (o)
+    {
+        PyObject* r = PyObject_Str(o);
+        if (r)
+            s = PyUnicode_AsUTF8AndSize(r, NULL);
+    }
+    return s;
+}
+
+} // unnamed namespace
+
 // http://www.linuxjournal.com/article/3641
 // http://www.codeproject.com/Articles/11805/Embedding-Python-in-C-C-Part-I
 // http://stackoverflow.com/questions/6596016/python-threads-in-c
@@ -199,9 +218,9 @@ std::string getTraceback()
             PyObject* r = PyObject_Repr(l);
             if (!r)
                 throw pdal::pdal_error("unable to get repr in getTraceback");
-            Py_ssize_t size;
-            const char *d = PyUnicode_AsUTF8AndSize(r, &size);
-            mssg << d;
+            const char *d = PyUnicode_AsUTF8AndSize(r, NULL);
+            if (d)
+                mssg << d;
         }
 
         // clean up
@@ -213,9 +232,9 @@ std::string getTraceback()
         PyObject* r = PyObject_Repr(value);
         if (!r)
             throw pdal::pdal_error("couldn't make string representation of traceback value");
-        Py_ssize_t size;
-        const char *d = PyUnicode_AsUTF8AndSize(r, &size);
-        mssg << d;
+        const char *d = PyUnicode_AsUTF8AndSize(r, NULL);
+        if (d)
+            mssg << d;
     }
     else
         mssg << "unknown error that we are unable to get a traceback for."
@@ -231,21 +250,6 @@ std::string getTraceback()
 // Returns a new reference.
 PyObject *fromMetadata(MetadataNode m)
 {
-    std::string name = m.name();
-    std::string value = m.value();
-    std::string type = m.type();
-    std::string description = m.description();
-
-    MetadataNodeList children = m.children();
-    PyObject *submeta(0);
-    if (children.size())
-    {
-        submeta = PyList_New(0);
-        for (MetadataNode& child : children)
-            PyList_Append(submeta, fromMetadata(child));
-    }
-    PyObject *data = PyDict_New();
-
     auto getString = [](const std::string& s)
     {
         PyObject *o = PyUnicode_FromString(s.data());
@@ -254,68 +258,59 @@ PyObject *fromMetadata(MetadataNode m)
         return o;
     };
 
-    PyDict_SetItemString(data, "name", getString(name));
-    PyDict_SetItemString(data, "value", getString(value));
-    PyDict_SetItemString(data, "type", getString(value));
-    PyDict_SetItemString(data, "description", getString(description));
+    PyObject *data = PyDict_New();
+
+    PyDict_SetItemString(data, "name", getString(m.name()));
+    PyDict_SetItemString(data, "value", getString(m.value()));
+    PyDict_SetItemString(data, "type", getString(m.type()));
+    PyDict_SetItemString(data, "description", getString(m.description()));
 
+    MetadataNodeList children = m.children();
     if (children.size())
+    {
+        PyObject *submeta = PyList_New(0);
+        for (MetadataNode& child : children)
+            PyList_Append(submeta, fromMetadata(child));
         PyDict_SetItemString(data, "children", submeta);
+    }
+
     return data;
 }
 
-std::string readPythonString(PyObject* dict, const std::string& key)
-{
-    std::stringstream ss;
 
-    PyObject* o = PyDict_GetItemString(dict, key.c_str());
-    if (!o)
-    {
-        std::stringstream oss;
-        oss << "Unable to get dictionary item '" << key << "'";
-        throw pdal_error(oss.str());
-    }
-
-    PyObject* r = PyObject_Str(o);
-    if (!r)
-        throw pdal::pdal_error("unable to get repr in readPythonString");
-    Py_ssize_t size;
-    const char *d = PyUnicode_AsUTF8AndSize(r, &size);
-    ss << d;
-
-    return ss.str();
-}
 void addMetadata(PyObject *dict, MetadataNode m)
 {
     if (!dict)
-    {
         return;
-    }
 
     if (!PyDict_Check(dict))
-        throw pdal::pdal_error("'metadata' member must be a dictionary!");
+        throw pdal::pdal_error("Output metadata must be in a dictionary.");
 
     std::string name = readPythonString(dict, "name");
     std::string value = readPythonString(dict, "value");
-
     std::string type = readPythonString(dict, "type");
+    std::string description = readPythonString(dict, "description");
+    if (name.empty())
+        return;
     if (type.empty())
         type = Metadata::inferType(value);
 
-    std::string description = readPythonString(dict, "description");
-
+    m.addWithType(name, value, type, description);
     PyObject *submeta = PyDict_GetItemString(dict, "children");
     if (submeta)
     {
         if (!PyList_Check(submeta))
-            throw pdal::pdal_error("'children' metadata member must be a list!");
-
-        for (Py_ssize_t i = 0; i < PyList_Size(submeta); ++i)
+            throw pdal::pdal_error("Ouput metadata 'children' must be a list.");
+        Py_ssize_t size = PyList_Size(submeta);
+        if (size)
         {
-            PyObject* p = PyList_GetItem(submeta, i);
-            addMetadata(p, m);
+            m = m.add("children");
+            for (Py_ssize_t i = 0; i < PyList_Size(submeta); ++i)
+            {
+                PyObject* p = PyList_GetItem(submeta, i);
+                addMetadata(p, m);
+            }
         }
-        MetadataNode child = m.addWithType(name, value, type, description);
     }
 }
 

pdal/plang/Invocation.cpp

@@ -473,11 +473,10 @@ void Invocation::extractData(PointViewPtr& view, PyObject *arrays)
 
 void Invocation::extractMetadata(MetadataNode stageMetadata)
 {
-    PyObject *key = PyUnicode_FromString("metadata");
+    PyObject *key = PyUnicode_FromString("out_metadata");
     PyObject *dictionary = PyModule_GetDict(m_module);
     PyObject *pyMeta = PyDict_GetItem(dictionary, key);
-    if (pyMeta)
-        addMetadata(pyMeta, stageMetadata);
+    addMetadata(pyMeta, stageMetadata);
     Py_DECREF(key);
 }
 

pdal/test/PythonFilterTest.cpp

@@ -223,9 +223,9 @@ TEST_F(PythonFilterTest, metadata)
         "import sys\n"
         "import redirector\n"
         "def myfunc(ins,outs):\n"
-        "  global metadata\n"
+        "  global out_metadata\n"
         "  #print('before', globals(),  file=sys.stderr,)\n"
-        "  metadata = {'name': 'root', 'value': 'a string', 'type': 'string', 'description': 'a description', 'children': [{'name': 'filters.python', 'value': 52, 'type': 'integer', 'description': 'a filter description', 'children': []}, {'name': 'readers.faux', 'value': 'another string', 'type': 'string', 'description': 'a reader description', 'children': []}]}\n"
+        "  out_metadata = {'name': 'root', 'value': 'a string', 'type': 'string', 'description': 'a description', 'children': [{'name': 'somechildren', 'value': 52, 'type': 'integer', 'description': 'a filter description', 'children': []}, {'name': 'otherchildren', 'value': 'another string', 'type': 'string', 'description': 'a reader description', 'children': []}]}\n"
         " # print ('schema', schema, file=sys.stderr,)\n"
         "  return True\n"
     );
@@ -250,10 +250,11 @@ TEST_F(PythonFilterTest, metadata)
     MetadataNode m = table.metadata();
     m = m.findChild("filters.python");
     MetadataNodeList l = m.children();
-    EXPECT_EQ(l.size(), 3u);
-    EXPECT_EQ(l[0].name(), "filters.python");
-    EXPECT_EQ(l[0].value(), "52");
-    EXPECT_EQ(l[0].description(), "a filter description");
+    EXPECT_EQ(l.size(), 2u);
+    m = m.findChild("children");
+    EXPECT_EQ(m.children().size(), 2u);
+    m = m.findChild("somechildren");
+    EXPECT_EQ(m.value(), "52");
 }
 
 TEST_F(PythonFilterTest, pdalargs)

test/test_pipeline.py

@@ -228,7 +228,7 @@ class TestDimensions(PDALTest):
     def test_fetch_dimensions(self):
         """Ask PDAL for its valid dimensions list"""
         dims = pdal.dimensions
-        self.assertLess(len(dims), 100)
+        self.assertLess(len(dims), 120)
         self.assertGreater(len(dims), 71)
 
 class TestMesh(PDALTest):