3.2.0 release prep (#127)

hobu · web-flow · commit ee07fc2e7e8c · 2022-09-28T12:22:49.000-05:00
* tweak Development.Module support to not include Apple * force cmake > 3.11 for building with pip * bump CMAKE_CXX_STANDARD to c++17 to match PDAL * fetch drivers and options directly from the library instead of shelling out to 'pdal' command * bump version no to 3.2.0 * bypass DEVELOPMENT_COMPONENT for now * compare_structured_arrays method for testing array equality now that numpy 1.23.0's deprecation has taken effect * PDAL 2.4 uses / separators all the time * Implement #123, rename _get_json to toJSON * Revert "Implement #123, rename _get_json to toJSON" This reverts commit f35c3dd. * no more editable installs * Revert "Revert "Implement #123, rename _get_json to toJSON"" This reverts commit 085bce2. * fix #119, put back json alias to Pipeline * fix #125 and set 'type' for getJSON serialization * regularize access to a pipeline's srswkt2 to fix #112 * update CHANGES * changes note * updates and changes as suggested by review
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -44,11 +44,11 @@ jobs:
           python=${{ matrix.python-version }}
 
     - name: Install python-pdal
-      run: pip install -e .
+      run: pip install .
 
     - name: Install python-pdal-plugins
       working-directory: ./plugins
-      run: pip install -e .
+      run: pip install .
 
     - name: Test
       run: |
diff --git a/CHANGES.txt b/CHANGES.txt
@@ -1,6 +1,30 @@
 Changes
 --------------------------------------------------------------------------------
 
+3.2.0
+................................................................................
+
+* PDAL base library 2.4.0+ is required
+
+* CMake project name updated to pdal-python
+
+* `srswkt2` property added to allow fetching of SRS info
+
+* pip builds require cmake >= 3.11
+
+* CMAKE_CXX_STANDARD set to c++17 to match PDAL 2.4.x
+
+* Driver and options *actually* uses the library instead of
+  shelling out to `pdal` application :)
+
+* _get_json renamed to toJSON and made public
+
+* Fix #119, 'json' optional kwarg put back for now
+
+* DEVELOPMENT_COMPONENT in CMake FindPython skipped on OSX
+
+* Make sure 'type' gets set when serializing to JSON
+
 3.1.0
 ................................................................................
 
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -1,7 +1,7 @@
 cmake_minimum_required(VERSION 3.11.0)
 project(pdal-python)
 
-set(CMAKE_CXX_STANDARD 11)
+set(CMAKE_CXX_STANDARD 17)
 set(CMAKE_CXX_STANDARD_REQUIRED ON)
 set(CMAKE_CXX_EXTENSIONS OFF)
 set(CMAKE_BUILD_TYPE "Release")
@@ -13,14 +13,14 @@ set(Python3_FIND_FRAMEWORK "LAST")
 
 # Development vs. Development.Module
 # https://cmake.org/cmake/help/latest/module/FindPython3.html?highlight=Development.Module
-if (${CMAKE_VERSION} VERSION_GREATER_EQUAL "3.18.0")
+if (${CMAKE_VERSION} VERSION_GREATER_EQUAL "3.18.0" AND NOT APPLE)
     set(DEVELOPMENT_COMPONENT "Development.Module")
 else()
     set(DEVELOPMENT_COMPONENT "Development")
 endif()
 
 # find Python3
-find_package(Python3 COMPONENTS Interpreter "${DEVELOPMENT_COMPONENT}" NumPy REQUIRED)
+find_package(Python3 COMPONENTS Interpreter Development NumPy REQUIRED)
 
 # find PDAL. Require 2.1+
 find_package(PDAL 2.4 REQUIRED)
diff --git a/pdal/PyPipeline.cpp b/pdal/PyPipeline.cpp
@@ -80,6 +80,17 @@ point_count_t PipelineExecutor::execute()
     return count;
 }
 
+std::string PipelineExecutor::getSrsWKT2() const
+{
+    std::string output("");
+    pdal::PointTableRef pointTable = m_manager.pointTable();
+
+
+    pdal::SpatialReference srs = pointTable.spatialReference();
+    output = srs.getWKT();
+
+    return output;
+}
 
 point_count_t PipelineExecutor::executeStream(point_count_t streamLimit)
 {
diff --git a/pdal/PyPipeline.hpp b/pdal/PyPipeline.hpp
@@ -61,6 +61,8 @@ class PDAL_DLL PipelineExecutor {
     std::string getMetadata() const;
     std::string getQuickInfo() const;
     std::string getSchema() const;
+    std::string getSrsWKT2() const;
+    PipelineManager const& getManager() const { return m_manager; }
     std::string getLog() const { return m_logStream.str(); }
 
 protected:
diff --git a/pdal/__init__.py b/pdal/__init__.py
@@ -1,4 +1,4 @@
-__version__ = "3.1.2"
+__version__ = "3.2.0"
 __all__ = ["Pipeline", "Stage", "Reader", "Filter", "Writer", "dimensions", "info"]
 
 from . import libpdalpython
diff --git a/pdal/drivers.py b/pdal/drivers.py
@@ -63,33 +63,9 @@ def factory(self) -> Callable[..., Stage]:
 
 def inject_pdal_drivers() -> None:
 
-#     drivers = libpdalpython.getDrivers()
-#
-#     options = libpdalpython.getOptions()
+    drivers = libpdalpython.getDrivers()
+    options = libpdalpython.getOptions()
 
-    drivers = json.loads(
-        subprocess.run(["pdal", "--drivers", "--showjson"], capture_output=True).stdout
-    )
-    options = dict(
-        json.loads(
-            subprocess.run(
-                ["pdal", "--options", "all", "--showjson"], capture_output=True
-            ).stdout
-        )
-    )
-
-    command = 'pdal --options all --showjson'
-
-    p = subprocess.run(shlex.split(command),
-                    encoding='utf-8',
-                    capture_output=True)
-    output, error = p.stdout, p.stderr
-    if p.returncode:
-        print (f'return code {p.returncode} error: "{error}"')
-        if error:
-            raise RuntimeError(f"Unable to run pdal --options with error '{error}'")
-
-    options = dict( json.loads( output ))
     streamable = []
     for d in drivers:
         name = d["name"]
diff --git a/pdal/libpdalpython.cpp b/pdal/libpdalpython.cpp
@@ -57,18 +57,17 @@ namespace pdal {
 
     };
 
-   std::vector<py::object> getOptions() {
+   py::object getOptions() {
         py::gil_scoped_acquire acquire;
         py::object json = py::module_::import("json");
-        std::vector<py::object> stageOptions;
+        py::dict stageOptions;
 
         pdal::StageFactory f;
         pdal::PluginManager<pdal::Stage>::loadAll();
         pdal::StringList stages = pdal::PluginManager<pdal::Stage>::names();
 
         for (auto name : stages)
         {
-            if ( name == "filters.info" ) continue;
             pdal::Stage *s = f.createStage(name);
             pdal::ProgramArgs args;
             s->addAllArgs(args);
@@ -82,13 +81,12 @@ namespace pdal {
             try {
                 j = json.attr("loads")(pystring);
             } catch (py::error_already_set &e) {
-                std::cout << "failed:" << name << "'" << ostr.str() << "'" <<std::endl;
+                std::cerr << "failed:" << name << "'" << ostr.str() << "'" <<std::endl;
                 continue; // skip this one because we can't parse it
             }
 
             f.destroyStage(s);
-            py::list l = py::make_tuple( name, pystring);
-            stageOptions.push_back(std::move(l));
+            stageOptions[pybind11::cast(name)] = std::move(j);
        }
         return stageOptions;
 
@@ -178,6 +176,7 @@ namespace pdal {
         std::string getLog() { return getExecutor()->getLog(); }
 
         std::string getPipeline() { return getExecutor()->getPipeline(); }
+        std::string getSrsWKT2() { return getExecutor()->getSrsWKT2(); }
 
         py::object getQuickInfo() {
             py::gil_scoped_acquire acquire;
@@ -220,7 +219,7 @@ namespace pdal {
         }
 
         std::string getJson() const {
-            PYBIND11_OVERRIDE_PURE_NAME(std::string, Pipeline, "_get_json", getJson);
+            PYBIND11_OVERRIDE_PURE_NAME(std::string, Pipeline, "toJSON", getJson);
         }
 
         bool hasInputs() { return !_inputs.empty(); }
@@ -248,6 +247,7 @@ namespace pdal {
         .def("__next__", &PipelineIterator::executeNext)
         .def_property_readonly("log", &PipelineIterator::getLog)
         .def_property_readonly("schema", &PipelineIterator::getSchema)
+        .def_property_readonly("srswkt2", &PipelineIterator::getSrsWKT2)
         .def_property_readonly("pipeline", &PipelineIterator::getPipeline)
         .def_property_readonly("metadata", &PipelineIterator::getMetadata);
 
@@ -260,14 +260,15 @@ namespace pdal {
         .def_property("loglevel", &Pipeline::getLoglevel, &Pipeline::setLogLevel)
         .def_property_readonly("log", &Pipeline::getLog)
         .def_property_readonly("schema", &Pipeline::getSchema)
+        .def_property_readonly("srswkt2", &Pipeline::getSrsWKT2)
         .def_property_readonly("pipeline", &Pipeline::getPipeline)
         .def_property_readonly("quickinfo", &Pipeline::getQuickInfo)
         .def_property_readonly("metadata", &Pipeline::getMetadata)
         .def_property_readonly("arrays", &Pipeline::getArrays)
         .def_property_readonly("meshes", &Pipeline::getMeshes)
         .def_property_readonly("_has_inputs", &Pipeline::hasInputs)
         .def("_copy_inputs", &Pipeline::copyInputs)
-        .def("_get_json", &Pipeline::getJson)
+        .def("toJSON", &Pipeline::getJson)
         .def("_del_executor", &Pipeline::delExecutor);
     m.def("getInfo", &getInfo);
     m.def("getDrivers", &getDrivers);
diff --git a/pdal/pipeline.py b/pdal/pipeline.py
@@ -28,7 +28,13 @@ def __init__(
         spec: Union[None, str, Sequence[Stage]] = None,
         arrays: Sequence[np.ndarray] = (),
         loglevel: int = logging.ERROR,
+        json: Optional[str] = None
     ):
+        if json:
+            if spec and json:
+                raise ValueError("provide 'spec' or 'json' arguments, not both")
+            spec = json
+
         super().__init__()
         self._stages: List[Stage] = []
         if spec:
@@ -107,6 +113,9 @@ def get_meshio(self, idx: int) -> Optional[Mesh]:
         )
 
     def _get_json(self) -> str:
+        return self.toJSON()
+
+    def toJSON(self) -> str:
         options_list = []
         stage2tag: Dict[Stage, str] = {}
         stages = self._stages
@@ -121,6 +130,7 @@ def _get_json(self) -> str:
             if inputs:
                 options["inputs"] = inputs
             options_list.append(options)
+
         return json.dumps(options_list)
 
 
diff --git a/pyproject.toml b/pyproject.toml
@@ -1,2 +1,2 @@
 [build-system]
-requires = ["scikit-build", "cmake", "ninja", "numpy",  "pybind11[global]"]
+requires = ["scikit-build", "cmake>=3.11", "ninja", "numpy",  "pybind11[global]"]
diff --git a/test/test_pipeline.py b/test/test_pipeline.py
@@ -15,6 +15,13 @@ def a_filter(ins, outs):
     return True
 
 
+def compare_structured_arrays(arr1, arr2):
+    for field in arr1.dtype.names:
+        equal = np.all(np.equal(arr1[field], arr2[field]))
+        if not equal:
+            return False
+    return True
+
 def get_pipeline(filename):
     with open(os.path.join(DATADIRECTORY, filename), "r") as f:
         if filename.endswith(".json"):
@@ -135,8 +142,7 @@ def test_pipeline(self, filename):
         assert json.loads(r.pipeline) == {
             "pipeline": [
                 {
-                    # TODO: update this after https://github.com/PDAL/PDAL/issues/3574
-                    "filename": f"test/data{os.sep}1.2-with-color.las",
+                    "filename": "test/data/1.2-with-color.las",
                     "tag": "readers_las1",
                     "type": "readers.las",
                 },
@@ -364,6 +370,14 @@ def test_quickinfo(self):
         assert 'readers.las' in info.keys()
         assert info['readers.las']['num_points'] == 1065
 
+    def test_jsonkwarg(self):
+        pipeline = pdal.Reader("test/data/autzen-utm.las").pipeline().toJSON()
+        r = pdal.Pipeline(json=pipeline)
+        p = r.pipeline
+        assert 'readers.las' in p
+
+
+
 class TestArrayLoad:
     def test_merged_arrays(self):
         """Can we load data from a list of arrays to PDAL"""
@@ -441,6 +455,7 @@ def test_reference_counting(self):
         assert refcount == 1
 
 
+
 class TestMesh:
     @pytest.mark.parametrize("filename", ["sort.json", "sort.py"])
     def test_no_execute(self, filename):
@@ -496,7 +511,7 @@ def test_array(self, filename):
             arrays = list(r.iterator(chunk_size=100))
             assert len(arrays) == 11
             concat_array = np.concatenate(arrays)
-            np.testing.assert_array_equal(array, concat_array)
+            assert compare_structured_arrays(np.concatenate(arrays), concat_array)
 
     @pytest.mark.parametrize("filename", ["range.json", "range.py"])
     def test_StopIteration(self, filename):
@@ -550,9 +565,7 @@ def test_merged_arrays(self):
         non_streaming_array = np.concatenate(p.arrays)
         for chunk_size in range(5, 100, 5):
             streaming_arrays = list(p.iterator(chunk_size=chunk_size))
-            np.testing.assert_array_equal(
-                np.concatenate(streaming_arrays), non_streaming_array
-            )
+            assert compare_structured_arrays(np.concatenate(streaming_arrays), non_streaming_array)
 
     @pytest.mark.parametrize("filename", ["range.json", "range.py"])
     def test_premature_exit(self, filename):
@@ -562,9 +575,12 @@ def test_premature_exit(self, filename):
         assert len(r.arrays) == 1
         array = r.arrays[0]
 
+        # the dtype ordering of these arrays are not going to be the
+        # same. Use our testing method to compare them.
+
         for _ in range(10):
             for array2 in r.iterator(chunk_size=100):
-                np.testing.assert_array_equal(array2, array[: len(array2)])
+                assert compare_structured_arrays(array2, array[: len(array2)])
                 break
 
     @pytest.mark.parametrize("filename", ["range.json", "range.py"])

Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-__version__ = "3.1.2"`
	`1`	`+__version__ = "3.2.0"`
`2`	`2`	`__all__ = ["Pipeline", "Stage", "Reader", "Filter", "Writer", "dimensions", "info"]`
`3`	`3`
`4`	`4`	`from . import libpdalpython`
Original file line number	Diff line number	Diff line change
`@@ -1,2 +1,2 @@`
`1`	`1`	`[build-system]`
`2`		`-requires = ["scikit-build", "cmake", "ninja", "numpy", "pybind11[global]"]`
	`2`	`+requires = ["scikit-build", "cmake>=3.11", "ninja", "numpy", "pybind11[global]"]`