From af859ec135a436e054c9478013b45c3f347bd8fe Mon Sep 17 00:00:00 2001 From: Howard Butler Date: Wed, 24 Apr 2024 14:29:09 -0500 Subject: [PATCH 1/6] Set Numpy floor to 1.22 and NPY_NO_DEPRECATED_API NPY_1_22_API_VERSION --- CMakeLists.txt | 4 +++- pyproject.toml | 4 ++-- src/pdal/PyArray.hpp | 5 +++++ src/pdal/PyPipeline.hpp | 4 ++++ src/pdal/StreamableExecutor.cpp | 4 ++-- 5 files changed, 16 insertions(+), 5 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 8c2860ed..e22cd1a1 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,5 +1,7 @@ cmake_minimum_required(VERSION 3.11.0) -project(pdal-python VERSION) +project(pdal-python VERSION ${SKBUILD_PROJECT_VERSION} + DESCRIPTION "PDAL Python bindings" + HOMEPAGE_URL "https://github.com/PDAL/Python") set(CMAKE_CXX_STANDARD 17) set(CMAKE_CXX_STANDARD_REQUIRED ON) diff --git a/pyproject.toml b/pyproject.toml index 419a8b08..eca080ee 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -26,7 +26,7 @@ classifiers = [ ] dependencies = [ - "numpy" + "numpy >= 1.22" ] dynamic = ["version"] @@ -48,7 +48,7 @@ repository = "https://github.com/PDAL/Python" changelog = "https://github.com/PDAL/python/blob/main/README.rst" [build-system] -requires = ["scikit-build-core >= 0.9", "numpy", "pybind11[global]"] +requires = ["scikit-build-core >= 0.9", "numpy >= 1.22", "pybind11[global]"] build-backend = "scikit_build_core.build" diff --git a/src/pdal/PyArray.hpp b/src/pdal/PyArray.hpp index f0401f00..859965bc 100644 --- a/src/pdal/PyArray.hpp +++ b/src/pdal/PyArray.hpp @@ -35,7 +35,12 @@ #pragma once #include + +#define NPY_TARGET_VERSION NPY_1_22_API_VERSION +#define NPY_NO_DEPRECATED_API NPY_1_22_API_VERSION + #include + #include #include diff --git a/src/pdal/PyPipeline.hpp b/src/pdal/PyPipeline.hpp index c4e316c6..1e1edadc 100644 --- a/src/pdal/PyPipeline.hpp +++ b/src/pdal/PyPipeline.hpp @@ -35,6 +35,10 @@ #pragma once #include + +#define NPY_TARGET_VERSION NPY_1_22_API_VERSION +#define NPY_NO_DEPRECATED_API NPY_1_22_API_VERSION + #include namespace pdal diff --git a/src/pdal/StreamableExecutor.cpp b/src/pdal/StreamableExecutor.cpp index ef9950e2..c0edb95a 100644 --- a/src/pdal/StreamableExecutor.cpp +++ b/src/pdal/StreamableExecutor.cpp @@ -102,8 +102,8 @@ void PythonPointTable::py_resizeArray(point_count_t np) { if (src_idx != dest_idx) { - PyObject* src_item = PyArray_GETITEM(m_curArray, PyArray_GETPTR1(m_curArray, src_idx)); - PyArray_SETITEM(m_curArray, PyArray_GETPTR1(m_curArray, dest_idx), src_item); + PyObject* src_item = PyArray_GETITEM(m_curArray, (const char*) PyArray_GETPTR1(m_curArray, src_idx)); + PyArray_SETITEM(m_curArray, (char*) PyArray_GETPTR1(m_curArray, dest_idx), src_item); Py_XDECREF(src_item); } dest_idx++; From f1243ca3c80864d782d9110ae86ba8694797d7ac Mon Sep 17 00:00:00 2001 From: Howard Butler Date: Thu, 25 Apr 2024 08:59:14 -0500 Subject: [PATCH 2/6] set PY_ARRAY_UNIQUE_SYMBOL to PDAL_ARRAY_API and only import it in extension initialization. Guard against using the extension against any PDAL verisions < 2.6 --- src/pdal/PyArray.cpp | 3 --- src/pdal/PyArray.hpp | 4 ++++ src/pdal/PyPipeline.cpp | 16 +--------------- src/pdal/PyPipeline.hpp | 3 +++ src/pdal/StreamableExecutor.cpp | 6 ++++-- src/pdal/libpdalpython.cpp | 18 ++++++++++++++++++ 6 files changed, 30 insertions(+), 20 deletions(-) diff --git a/src/pdal/PyArray.cpp b/src/pdal/PyArray.cpp index 04c424bd..66b6bd45 100644 --- a/src/pdal/PyArray.cpp +++ b/src/pdal/PyArray.cpp @@ -92,9 +92,6 @@ std::string toString(PyObject *pname) Array::Array(PyArrayObject* array) : m_array(array), m_rowMajor(true) { - if (_import_array() < 0) - throw pdal_error("Could not import numpy.core.multiarray."); - Py_XINCREF(array); PyArray_Descr *dtype = PyArray_DTYPE(m_array); diff --git a/src/pdal/PyArray.hpp b/src/pdal/PyArray.hpp index 859965bc..e2da961f 100644 --- a/src/pdal/PyArray.hpp +++ b/src/pdal/PyArray.hpp @@ -39,9 +39,13 @@ #define NPY_TARGET_VERSION NPY_1_22_API_VERSION #define NPY_NO_DEPRECATED_API NPY_1_22_API_VERSION +#define NO_IMPORT_ARRAY +#define PY_ARRAY_UNIQUE_SYMBOL PDAL_ARRAY_API + #include #include +#include #include #include diff --git a/src/pdal/PyPipeline.cpp b/src/pdal/PyPipeline.cpp index 35859f06..85ea028a 100644 --- a/src/pdal/PyPipeline.cpp +++ b/src/pdal/PyPipeline.cpp @@ -211,16 +211,7 @@ std::string PipelineExecutor::getQuickInfo() const void PipelineExecutor::addArrayReaders(std::vector> arrays) { - // Make the symbols in pdal_base global so that they're accessible - // to PDAL plugins. Python dlopen's this extension with RTLD_LOCAL, - // which means that without this, symbols in libpdal_base aren't available - // for resolution of symbols on future runtime linking. This is an issue - // on Alpine and other Linux variants that don't use UNIQUE symbols - // for C++ template statics only. Without this, you end up with multiple - // copies of template statics. -#ifndef _WIN32 - ::dlopen("libpdal_base.so", RTLD_NOLOAD | RTLD_GLOBAL); -#endif + if (arrays.empty()) return; @@ -320,8 +311,6 @@ PyObject* buildNumpyDescriptor(PointLayoutPtr layout) PyArrayObject* viewToNumpyArray(PointViewPtr view) { - if (_import_array() < 0) - throw pdal_error("Could not import numpy.core.multiarray."); PyObject* dtype_dict = buildNumpyDescriptor(view->layout()); PyArray_Descr *dtype = nullptr; @@ -344,9 +333,6 @@ PyArrayObject* viewToNumpyArray(PointViewPtr view) PyArrayObject* meshToNumpyArray(const TriangularMesh* mesh) { - if (_import_array() < 0) - throw pdal_error("Could not import numpy.core.multiarray."); - // Build up a numpy dtype dictionary // // {'formats': ['f8', 'f8', 'f8', 'u2', 'u1', 'u1', 'u1', 'u1', 'u1', diff --git a/src/pdal/PyPipeline.hpp b/src/pdal/PyPipeline.hpp index 1e1edadc..c32abcfe 100644 --- a/src/pdal/PyPipeline.hpp +++ b/src/pdal/PyPipeline.hpp @@ -39,6 +39,9 @@ #define NPY_TARGET_VERSION NPY_1_22_API_VERSION #define NPY_NO_DEPRECATED_API NPY_1_22_API_VERSION +#define NO_IMPORT_ARRAY +#define PY_ARRAY_UNIQUE_SYMBOL PDAL_ARRAY_API + #include namespace pdal diff --git a/src/pdal/StreamableExecutor.cpp b/src/pdal/StreamableExecutor.cpp index c0edb95a..9f5b4b8b 100644 --- a/src/pdal/StreamableExecutor.cpp +++ b/src/pdal/StreamableExecutor.cpp @@ -35,6 +35,9 @@ #include "PyPipeline.hpp" #include "StreamableExecutor.hpp" +#define NO_IMPORT_ARRAY +#define PY_ARRAY_UNIQUE_SYMBOL PDAL_ARRAY_API + #include #include @@ -67,8 +70,7 @@ void PythonPointTable::finalize() // create dtype auto gil = PyGILState_Ensure(); - if (_import_array() < 0) - std::cerr << "Could not import array!\n"; + PyObject *dtype_dict = buildNumpyDescriptor(&m_layout); if (PyArray_DescrConverter(dtype_dict, &m_dtype) == NPY_FAIL) throw pdal_error("Unable to create numpy dtype"); diff --git a/src/pdal/libpdalpython.cpp b/src/pdal/libpdalpython.cpp index 7bd526a0..229f928d 100644 --- a/src/pdal/libpdalpython.cpp +++ b/src/pdal/libpdalpython.cpp @@ -7,6 +7,13 @@ #include #include +#define NPY_TARGET_VERSION NPY_1_22_API_VERSION +#define NPY_NO_DEPRECATED_API NPY_1_22_API_VERSION + +#define PY_ARRAY_UNIQUE_SYMBOL PDAL_ARRAY_API + +#include + #include "PyArray.hpp" #include "PyDimension.hpp" #include "PyPipeline.hpp" @@ -283,8 +290,12 @@ namespace pdal { int _loglevel; }; + + PYBIND11_MODULE(libpdalpython, m) { + _import_array(); + py::class_(m, "PipelineIterator") .def("__iter__", [](PipelineIterator &it) -> PipelineIterator& { return it; }) .def("__next__", &PipelineIterator::executeNext) @@ -294,6 +305,7 @@ namespace pdal { .def_property_readonly("pipeline", &PipelineIterator::getPipeline) .def_property_readonly("metadata", &PipelineIterator::getMetadata); + py::class_(m, "Pipeline") .def(py::init<>()) .def("execute", &Pipeline::execute) @@ -319,6 +331,12 @@ namespace pdal { m.def("getDimensions", &getDimensions); m.def("infer_reader_driver", &getReaderDriver); m.def("infer_writer_driver", &getWriterDriver); + + if (pdal::Config::versionMajor() < 2) + throw pybind11::import_error("PDAL version must be >= 2.6"); + + if (pdal::Config::versionMajor() == 2 && pdal::Config::versionMinor() < 6) + throw pybind11::import_error("PDAL version must be >= 2.6"); }; }; // namespace pdal From 9199711053b6b5a16889eb430b0b1329bb7e93f1 Mon Sep 17 00:00:00 2001 From: Howard Butler Date: Thu, 25 Apr 2024 09:08:01 -0500 Subject: [PATCH 3/6] debug PDAL_DRIVER_PATH issues --- .github/workflows/build.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 65f01ff3..8d1dd1de 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -57,8 +57,10 @@ jobs: - name: Test run: | - export PDAL_DRIVER_PATH=$(python -m pdal --pdal-plugin-path) - echo $PDAL_DRIVER_PATH + export PDAL_DRIVER_PATH=$(python -m pdal --pdal-driver-path) + export PDAL_PLUGIN_PATH=$(python -m pdal --pdal-plugin-path) + echo "PDAL_DRIVER_PATH $PDAL_DRIVER_PATH" + echo "PDAL_PLUGIN_PATH $PDAL_PLUGIN_PATH" pdal --drivers --debug py.test -v test/ From c0c4868998905df2491815a11987891f814d419f Mon Sep 17 00:00:00 2001 From: Howard Butler Date: Thu, 25 Apr 2024 09:15:42 -0500 Subject: [PATCH 4/6] bug in --print-plugin-path --- .github/workflows/build.yml | 1 + src/pdal/__main__.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 8d1dd1de..40b5b7a6 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -61,6 +61,7 @@ jobs: export PDAL_PLUGIN_PATH=$(python -m pdal --pdal-plugin-path) echo "PDAL_DRIVER_PATH $PDAL_DRIVER_PATH" echo "PDAL_PLUGIN_PATH $PDAL_PLUGIN_PATH" + python -m pdal pdal --drivers --debug py.test -v test/ diff --git a/src/pdal/__main__.py b/src/pdal/__main__.py index f45f3151..310e5872 100644 --- a/src/pdal/__main__.py +++ b/src/pdal/__main__.py @@ -23,7 +23,7 @@ def print_driver_path(args): print (os.environ['PDAL_DRIVER_PATH']) def print_plugin_path(args): - purelib = sysconfig.get_paths()["purelib"]+os.path.sep+"pdal" + purelib = sysconfig.get_paths()["purelib"]+os.path.sep if sys.platform == "linux" or sys.platform == "linux2": suffix = 'so' From 28570c3dbf883c41bfd361ee595f7bc63deaae08 Mon Sep 17 00:00:00 2001 From: Howard Butler Date: Thu, 25 Apr 2024 09:20:21 -0500 Subject: [PATCH 5/6] only one os.path.sep --- src/pdal/__main__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pdal/__main__.py b/src/pdal/__main__.py index 310e5872..7fa7d272 100644 --- a/src/pdal/__main__.py +++ b/src/pdal/__main__.py @@ -23,7 +23,7 @@ def print_driver_path(args): print (os.environ['PDAL_DRIVER_PATH']) def print_plugin_path(args): - purelib = sysconfig.get_paths()["purelib"]+os.path.sep + purelib = sysconfig.get_paths()["purelib"] if sys.platform == "linux" or sys.platform == "linux2": suffix = 'so' From 9ea9f248072a5a74eb81dff16ecc2fbc7ec967db Mon Sep 17 00:00:00 2001 From: Howard Butler Date: Thu, 25 Apr 2024 09:26:29 -0500 Subject: [PATCH 6/6] concat PDAL_DRIVER_PATH with PDAL_PLUGIN_PATH --- .github/workflows/build.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 40b5b7a6..42c82a78 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -61,6 +61,7 @@ jobs: export PDAL_PLUGIN_PATH=$(python -m pdal --pdal-plugin-path) echo "PDAL_DRIVER_PATH $PDAL_DRIVER_PATH" echo "PDAL_PLUGIN_PATH $PDAL_PLUGIN_PATH" + export PDAL_DRIVER_PATH=$PDAL_PLUGIN_PATH:$PDAL_DRIVER_PATH python -m pdal pdal --drivers --debug py.test -v test/