From d39a6c9c656f6d9dac1111505b8808f8a6f81844 Mon Sep 17 00:00:00 2001 From: Andrew Bell Date: Thu, 24 Jun 2021 11:41:37 -0400 Subject: [PATCH 1/9] Use out_metadata for stage metadata. --- pdal/plang/Environment.cpp | 90 ++++++++++++++++++-------------------- pdal/plang/Invocation.cpp | 7 ++- 2 files changed, 45 insertions(+), 52 deletions(-) diff --git a/pdal/plang/Environment.cpp b/pdal/plang/Environment.cpp index bf813b9f..251b0394 100644 --- a/pdal/plang/Environment.cpp +++ b/pdal/plang/Environment.cpp @@ -79,6 +79,29 @@ 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) + return s; + + PyObject* r = PyObject_Str(o); + if (r) + { + Py_ssize_t size; + s = PyUnicode_AsUTF8AndSize(r, &size); + } + + 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 @@ -231,21 +254,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 +262,54 @@ 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 = 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!"); - + throw pdal::pdal_error("Ouput metadata 'children' must be a list."); 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); } } diff --git a/pdal/plang/Invocation.cpp b/pdal/plang/Invocation.cpp index c74f9245..8268ae9c 100644 --- a/pdal/plang/Invocation.cpp +++ b/pdal/plang/Invocation.cpp @@ -384,7 +384,7 @@ PyObject *Invocation::prepareData(PointViewPtr& view) MetadataNode layoutMeta = view->layout()->toMetadata(); MetadataNode srsMeta = view->spatialReference().toMetadata(); - addGlobalObject(m_module, plang::fromMetadata(m_inputMetadata), "metadata"); + addGlobalObject(m_module, plang::fromMetadata(m_inputMetadata), "stage_metadata"); addGlobalObject(m_module, getPyJSON(m_pdalargs), "pdalargs"); addGlobalObject(m_module, getPyJSON(Utils::toJSON(layoutMeta)), "schema"); addGlobalObject(m_module, getPyJSON(Utils::toJSON(srsMeta)), @@ -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); } From 9c8541526dac1feba390744a6f87bc12d68ddf1d Mon Sep 17 00:00:00 2001 From: Andrew Bell Date: Thu, 24 Jun 2021 11:46:08 -0400 Subject: [PATCH 2/9] Revert input metadata object name. --- pdal/plang/Invocation.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pdal/plang/Invocation.cpp b/pdal/plang/Invocation.cpp index 8268ae9c..380ac132 100644 --- a/pdal/plang/Invocation.cpp +++ b/pdal/plang/Invocation.cpp @@ -384,7 +384,7 @@ PyObject *Invocation::prepareData(PointViewPtr& view) MetadataNode layoutMeta = view->layout()->toMetadata(); MetadataNode srsMeta = view->spatialReference().toMetadata(); - addGlobalObject(m_module, plang::fromMetadata(m_inputMetadata), "stage_metadata"); + addGlobalObject(m_module, plang::fromMetadata(m_inputMetadata), "metadata"); addGlobalObject(m_module, getPyJSON(m_pdalargs), "pdalargs"); addGlobalObject(m_module, getPyJSON(Utils::toJSON(layoutMeta)), "schema"); addGlobalObject(m_module, getPyJSON(Utils::toJSON(srsMeta)), From 72c5a5ddfa43daa4ba9c85e48df1ffb0c5c5e1df Mon Sep 17 00:00:00 2001 From: Andrew Bell Date: Thu, 24 Jun 2021 12:07:07 -0400 Subject: [PATCH 3/9] Change output metadata variable name. --- pdal/test/PythonFilterTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pdal/test/PythonFilterTest.cpp b/pdal/test/PythonFilterTest.cpp index 1db5b8ee..d3d40976 100644 --- a/pdal/test/PythonFilterTest.cpp +++ b/pdal/test/PythonFilterTest.cpp @@ -225,7 +225,7 @@ TEST_F(PythonFilterTest, metadata) "def myfunc(ins,outs):\n" " global 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': '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" " # print ('schema', schema, file=sys.stderr,)\n" " return True\n" ); From 77358e6690cc1fc0f9aab1fd6b9b1dcdf9c05db2 Mon Sep 17 00:00:00 2001 From: Andrew Bell Date: Thu, 24 Jun 2021 12:36:44 -0400 Subject: [PATCH 4/9] Test. --- pdal/test/PythonFilterTest.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pdal/test/PythonFilterTest.cpp b/pdal/test/PythonFilterTest.cpp index d3d40976..ffbbcc91 100644 --- a/pdal/test/PythonFilterTest.cpp +++ b/pdal/test/PythonFilterTest.cpp @@ -225,7 +225,7 @@ TEST_F(PythonFilterTest, metadata) "def myfunc(ins,outs):\n" " global metadata\n" " #print('before', globals(), file=sys.stderr,)\n" - " out_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" ); @@ -248,7 +248,7 @@ TEST_F(PythonFilterTest, metadata) PointLayoutPtr layout(table.layout()); MetadataNode m = table.metadata(); - m = m.findChild("filters.python"); + m = m.findChild("somechildren"); MetadataNodeList l = m.children(); EXPECT_EQ(l.size(), 3u); EXPECT_EQ(l[0].name(), "filters.python"); From ef9de96865beb0b55939d01a72e848c87f90358b Mon Sep 17 00:00:00 2001 From: Andrew Bell Date: Thu, 24 Jun 2021 12:53:15 -0400 Subject: [PATCH 5/9] Test. --- pdal/test/PythonFilterTest.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/pdal/test/PythonFilterTest.cpp b/pdal/test/PythonFilterTest.cpp index ffbbcc91..ccb6ff75 100644 --- a/pdal/test/PythonFilterTest.cpp +++ b/pdal/test/PythonFilterTest.cpp @@ -249,6 +249,7 @@ TEST_F(PythonFilterTest, metadata) PointLayoutPtr layout(table.layout()); MetadataNode m = table.metadata(); m = m.findChild("somechildren"); + Utils::toJSON(m, std::cerr); MetadataNodeList l = m.children(); EXPECT_EQ(l.size(), 3u); EXPECT_EQ(l[0].name(), "filters.python"); From 6413e09060d3c1b30ae166fbf0aae890b65f5f21 Mon Sep 17 00:00:00 2001 From: Andrew Bell Date: Fri, 25 Jun 2021 08:44:59 -0400 Subject: [PATCH 6/9] Fix metadata test. --- pdal/plang/Environment.cpp | 13 +++++++++---- pdal/test/PythonFilterTest.cpp | 14 +++++++------- 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/pdal/plang/Environment.cpp b/pdal/plang/Environment.cpp index 251b0394..63f92210 100644 --- a/pdal/plang/Environment.cpp +++ b/pdal/plang/Environment.cpp @@ -299,16 +299,21 @@ void addMetadata(PyObject *dict, MetadataNode m) if (type.empty()) type = Metadata::inferType(value); - m = m.addWithType(name, value, type, description); + m.addWithType(name, value, type, description); PyObject *submeta = PyDict_GetItemString(dict, "children"); if (submeta) { if (!PyList_Check(submeta)) throw pdal::pdal_error("Ouput metadata 'children' must be a list."); - for (Py_ssize_t i = 0; i < PyList_Size(submeta); ++i) + 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); + } } } } diff --git a/pdal/test/PythonFilterTest.cpp b/pdal/test/PythonFilterTest.cpp index ccb6ff75..d114ec93 100644 --- a/pdal/test/PythonFilterTest.cpp +++ b/pdal/test/PythonFilterTest.cpp @@ -223,7 +223,7 @@ 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" " 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" @@ -248,13 +248,13 @@ TEST_F(PythonFilterTest, metadata) PointLayoutPtr layout(table.layout()); MetadataNode m = table.metadata(); - m = m.findChild("somechildren"); - Utils::toJSON(m, std::cerr); + 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) From a791ce3a4fab7ea04eb3edb63d1d74a0a15fe202 Mon Sep 17 00:00:00 2001 From: Andrew Bell Date: Fri, 25 Jun 2021 09:13:05 -0400 Subject: [PATCH 7/9] More dimensions. --- test/test_pipeline.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_pipeline.py b/test/test_pipeline.py index eb73d346..0113428c 100644 --- a/test/test_pipeline.py +++ b/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): From f726dc4c8ba1d656847d0ca27e617972dda188c1 Mon Sep 17 00:00:00 2001 From: Andrew Bell Date: Fri, 25 Jun 2021 10:00:41 -0400 Subject: [PATCH 8/9] Don't mess with string size if we don't care. --- pdal/plang/Environment.cpp | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/pdal/plang/Environment.cpp b/pdal/plang/Environment.cpp index 63f92210..02968823 100644 --- a/pdal/plang/Environment.cpp +++ b/pdal/plang/Environment.cpp @@ -92,10 +92,7 @@ std::string readPythonString(PyObject* dict, const std::string& key) PyObject* r = PyObject_Str(o); if (r) - { - Py_ssize_t size; - s = PyUnicode_AsUTF8AndSize(r, &size); - } + s = PyUnicode_AsUTF8AndSize(r, NULL); return s; } @@ -222,9 +219,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 @@ -236,9 +233,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." From ca02b0385929fefa2be02c9ccd5df25c9e77b0d8 Mon Sep 17 00:00:00 2001 From: Andrew Bell Date: Fri, 25 Jun 2021 10:03:53 -0400 Subject: [PATCH 9/9] Simplify. --- pdal/plang/Environment.cpp | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/pdal/plang/Environment.cpp b/pdal/plang/Environment.cpp index 02968823..fbded805 100644 --- a/pdal/plang/Environment.cpp +++ b/pdal/plang/Environment.cpp @@ -87,13 +87,12 @@ std::string readPythonString(PyObject* dict, const std::string& key) std::string s; PyObject* o = PyDict_GetItemString(dict, key.c_str()); - if (!o) - return s; - - PyObject* r = PyObject_Str(o); - if (r) - s = PyUnicode_AsUTF8AndSize(r, NULL); - + if (o) + { + PyObject* r = PyObject_Str(o); + if (r) + s = PyUnicode_AsUTF8AndSize(r, NULL); + } return s; }