Merge pull request #89 from TileDB-Inc/refactor-extension

Extension refactoring

Author: abellgithub

.github/workflows/build.yml

@@ -25,33 +25,27 @@ jobs:
     - uses: actions/checkout@v2
     - uses: conda-incubator/setup-miniconda@v2
       with:
-        auto-update-conda: true
         python-version: ${{ matrix.python-version }}
+        mamba-version: "*"
         activate-environment: test
         channels: conda-forge
         auto-activate-base: false
 
     - name: Linux dependencies
       if: matrix.os == 'ubuntu-latest'
       shell: bash -l {0}
-      run: |
-          sudo apt-get install gcc-multilib -y
+      run: sudo apt-get install gcc-multilib -y
 
     - name: Dependencies
       shell: bash -l {0}
-      run: |
-        conda install --yes --quiet -c conda-forge scikit-build numpy python=${{ matrix.python-version }} compilers cython ninja pdal pytest conda-build -y
+      run: mamba install --yes --quiet -c conda-forge scikit-build numpy python=${{ matrix.python-version }} cython pdal pytest
 
     - name: Install
       shell: bash -l {0}
       run: |
         python setup.py install -- -G "Ninja" -DWITH_TESTS=ON
-
         pdal --drivers
 
-
-#        pip install  . --build-option="-DWITH_TESTS=ON"
-
     - name: Test
       shell: bash -l {0}
       run: |
@@ -63,7 +57,6 @@ jobs:
         $SKPATH/pdal_io_numpy_test
         py.test
 
-
   windows:
     name: ${{ matrix.os }} - ${{ matrix.python-version }}
 
@@ -79,21 +72,17 @@ jobs:
     - uses: conda-incubator/setup-miniconda@v2
       with:
         channels: conda-forge
-        auto-update-conda: true
         python-version: ${{ matrix.python-version }}
 
-
     - name: Dependencies
       shell: cmd /C CALL "{0}"
       run: |
-
         call conda activate test
-        conda install --yes --quiet -c conda-forge scikit-build numpy python=${{ matrix.python-version }} compilers cython ninja pdal pytest conda-build -y
+        conda install --yes --quiet -c conda-forge scikit-build numpy python=${{ matrix.python-version }} cython pdal pytest
 
     - name: Install
       shell: cmd /C CALL "{0}"
       run: |
-
         call conda activate test
         where python
         python setup.py develop -- -G "Ninja" -DWITH_TESTS=ON
@@ -127,13 +116,12 @@ jobs:
     - uses: conda-incubator/setup-miniconda@v2
       with:
         channels: conda-forge
-        auto-update-conda: true
         python-version: ${{ matrix.python-version }}
+        mamba-version: "*"
 
     - name: Dependencies
       shell: bash -l {0}
-      run: |
-        conda install --yes --quiet -c conda-forge scikit-build numpy python=${{ matrix.python-version }} compilers cython ninja pdal pytest conda-build -y
+      run: mamba install --yes --quiet -c conda-forge scikit-build numpy python=${{ matrix.python-version }} cython pdal pytest
 
     - name: sdist
       shell: bash -l {0}

pdal/CMakeLists.txt

@@ -1,8 +1,6 @@
 set(EXTENSION_SRC
     PyArray.cpp
     PyArray.hpp
-    PyMesh.cpp
-    PyMesh.hpp
     PyDimension.hpp
     PyPipeline.cpp
     PyPipeline.hpp)

pdal/PyArray.cpp

@@ -34,7 +34,6 @@
 
 #include "PyArray.hpp"
 #include <pdal/io/MemoryViewReader.hpp>
-
 #include <numpy/arrayobject.h>
 
 namespace pdal
@@ -90,11 +89,6 @@ std::string toString(PyObject *pname)
 
 } // unnamed namespace
 
-Array::Array() : m_array(nullptr)
-{
-    if (_import_array() < 0)
-        throw pdal_error("Could not import numpy.core.multiarray.");
-}
 
 Array::Array(PyArrayObject* array) : m_array(array), m_rowMajor(true)
 {
@@ -163,131 +157,21 @@ Array::Array(PyArrayObject* array) : m_array(array), m_rowMajor(true)
 
 Array::~Array()
 {
-    if (m_array)
-        Py_XDECREF((PyObject *)m_array);
+    Py_XDECREF(m_array);
 }
 
 
-void Array::update(PointViewPtr view)
-{
-    if (m_array)
-        Py_XDECREF((PyObject *)m_array);
-    m_array = nullptr;  // Just in case of an exception.
-
-    Dimension::IdList dims = view->dims();
-    npy_intp size = view->size();
-
-    PyObject *dtype_dict = (PyObject*)buildNumpyDescription(view);
-    if (!dtype_dict)
-        throw pdal_error("Unable to build numpy dtype "
-                "description dictionary");
-
-    PyArray_Descr *dtype = nullptr;
-    if (PyArray_DescrConverter(dtype_dict, &dtype) == NPY_FAIL)
-        throw pdal_error("Unable to build numpy dtype");
-    Py_XDECREF(dtype_dict);
-
-    // This is a 1 x size array.
-    m_array = (PyArrayObject *)PyArray_NewFromDescr(&PyArray_Type, dtype,
-            1, &size, 0, nullptr, NPY_ARRAY_CARRAY, nullptr);
-
-    // copy the data
-    DimTypeList types = view->dimTypes();
-    for (PointId idx = 0; idx < view->size(); idx++)
-    {
-        char *p = (char *)PyArray_GETPTR1(m_array, idx);
-        view->getPackedPoint(types, idx, p);
-    }
-}
-
-
-//ABELL - Who's responsible for incrementing the ref count?
-PyArrayObject *Array::getPythonArray() const
-{
-    return m_array;
-}
-
-PyObject* Array::buildNumpyDescription(PointViewPtr view) const
-{
-    // Build up a numpy dtype dictionary
-    //
-    // {'formats': ['f8', 'f8', 'f8', 'u2', 'u1', 'u1', 'u1', 'u1', 'u1',
-    //              'f4', 'u1', 'u2', 'f8', 'u2', 'u2', 'u2'],
-    // 'names': ['X', 'Y', 'Z', 'Intensity', 'ReturnNumber',
-    //           'NumberOfReturns', 'ScanDirectionFlag', 'EdgeOfFlightLine',
-    //           'Classification', 'ScanAngleRank', 'UserData',
-    //           'PointSourceId', 'GpsTime', 'Red', 'Green', 'Blue']}
-    //
-
-    Dimension::IdList dims = view->dims();
-
-    PyObject* dict = PyDict_New();
-    PyObject* sizes = PyList_New(dims.size());
-    PyObject* formats = PyList_New(dims.size());
-    PyObject* titles = PyList_New(dims.size());
-
-    for (size_t i = 0; i < dims.size(); ++i)
-    {
-        Dimension::Id id = dims[i];
-        Dimension::Type t = view->dimType(id);
-        npy_intp stride = view->dimSize(id);
-
-        std::string name = view->dimName(id);
-
-        std::string kind("i");
-        Dimension::BaseType b = Dimension::base(t);
-        if (b == Dimension::BaseType::Unsigned)
-            kind = "u";
-        else if (b == Dimension::BaseType::Signed)
-            kind = "i";
-        else if (b == Dimension::BaseType::Floating)
-            kind = "f";
-        else
-            throw pdal_error("Unable to map kind '" + kind  +
-                "' to PDAL dimension type");
-
-        std::stringstream oss;
-        oss << kind << stride;
-        PyObject* pySize = PyLong_FromLong(stride);
-        PyObject* pyTitle = PyUnicode_FromString(name.c_str());
-        PyObject* pyFormat = PyUnicode_FromString(oss.str().c_str());
-
-        PyList_SetItem(sizes, i, pySize);
-        PyList_SetItem(titles, i, pyTitle);
-        PyList_SetItem(formats, i, pyFormat);
-    }
-
-    PyDict_SetItemString(dict, "names", titles);
-    PyDict_SetItemString(dict, "formats", formats);
-
-    return dict;
-}
-
-bool Array::rowMajor() const
-{
-    return m_rowMajor;
-}
-
-Array::Shape Array::shape() const
-{
-    return m_shape;
-}
-
-const Array::Fields& Array::fields() const
-{
-    return m_fields;
-}
-
 ArrayIter& Array::iterator()
 {
-    ArrayIter *it = new ArrayIter(*this);
+    ArrayIter *it = new ArrayIter(m_array);
     m_iterators.push_back(std::unique_ptr<ArrayIter>(it));
     return *it;
 }
 
-ArrayIter::ArrayIter(Array& array)
+
+ArrayIter::ArrayIter(PyArrayObject* np_array)
 {
-    m_iter = NpyIter_New(array.getPythonArray(),
+    m_iter = NpyIter_New(np_array,
         NPY_ITER_EXTERNAL_LOOP | NPY_ITER_READONLY | NPY_ITER_REFS_OK,
         NPY_KEEPORDER, NPY_NO_CASTING, NULL);
     if (!m_iter)
@@ -324,16 +208,6 @@ ArrayIter& ArrayIter::operator++()
     return *this;
 }
 
-ArrayIter::operator bool () const
-{
-    return !m_done;
-}
-
-char * ArrayIter::operator * () const
-{
-    return *m_data;
-}
-
 } // namespace python
 } // namespace pdal
 

pdal/PyArray.hpp

@@ -34,12 +34,9 @@
 
 #pragma once
 
-#include <numpy/ndarraytypes.h>
-
 #include <pdal/PointView.hpp>
 #include <pdal/io/MemoryViewReader.hpp>
-
-#include <utility>
+#include <numpy/ndarraytypes.h>
 
 namespace pdal
 {
@@ -48,51 +45,42 @@ namespace python
 
 class ArrayIter;
 
+
 class PDAL_DLL Array
 {
 public:
     using Shape = std::array<size_t, 3>;
     using Fields = std::vector<MemoryViewReader::Field>;
 
-    // Create an array for reading data from PDAL.
-    Array();
-
-    // Create an array for writing data to PDAL.
     Array(PyArrayObject* array);
-
     ~Array();
-    void update(PointViewPtr view);
-    PyArrayObject *getPythonArray() const;
-    bool rowMajor() const;
-    Shape shape() const;
-    const Fields& fields() const;
-    ArrayIter& iterator();
 
+    bool rowMajor() const { return m_rowMajor; };
+    Shape shape() const { return m_shape; }
+    const Fields& fields() const { return m_fields; };
+    ArrayIter& iterator();
 
 private:
-    inline PyObject* buildNumpyDescription(PointViewPtr view) const;
-
-
     PyArrayObject* m_array;
-    Array& operator=(Array const& rhs);
     Fields m_fields;
     bool m_rowMajor;
     Shape m_shape {};
     std::vector<std::unique_ptr<ArrayIter>> m_iterators;
 };
 
+
 class ArrayIter
 {
 public:
     ArrayIter(const ArrayIter&) = delete;
     ArrayIter() = delete;
 
-    ArrayIter(Array& array);
+    ArrayIter(PyArrayObject*);
     ~ArrayIter();
 
     ArrayIter& operator++();
-    operator bool () const;
-    char *operator * () const;
+    operator bool () const { return !m_done; }
+    char* operator*() const { return *m_data; }
 
 private:
     NpyIter *m_iter;