Repository navigation
Conversation
iccir
force-pushed
the
pybind-mplarray-initial
branch
from
October 23, 2026 15:13
41da0ea to
c82b28b
Compare
Contributor
Author
|
I'm going to go ahead and mark this as "Ready for review". I believe that the Windows test failure is due to a timeout in the test and AppVeyor didn't make it to the finish line in time. |
iccir
marked this pull request as ready for review
October 23, 2026 16:23
…pybind-mplarray-initial
2 tasks done
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR summary
This pull request adds a new
mpl::arrayC++ class as well asmpl::deferto mplutils.h. It changes the qhull module to use these instead of pybind11'spy::array. Additional modules will be ported in the future.ndarray Overview
nanobind provides an ndarray class for NumPy array access. While similar to pybind11's array class, it is not a drop-in replacement.
nb::ndarraydoes not automatically convert Python sequences.nb::ndarrayrequires aconstqualifier on scalar types in order to accept read-only arrays.nb::ndarrayhas a different API.mutable_data()is simplydata()on a non-const array. There is nounchecked()orunchecked_mutable(), etc.nb::ndarrayalso provides shape constraints. In theory, these are great for code readability, as we can indicate an(N, 3)integer array as follows:Unfortunately, nanobind is exceedingly vague when it fails to match a type signature to a bound function. When porting modules, you often end up with:
The root cause can be your scalar is
intinstead ofconst int. Or it can be that the shape constraint is mismatched. You don't know. This is especially fun when your function takes 5+ differentndarraytypes and nanobind doesn't indicate which one caused the failure.nanobind also requires that all typecasters be
noexceptand return a simplefalsein the event of an error. Hence, there is no way to provide detailed information on why the type caster failed.Typical Recommendation
The typical recommendation to the above problem is: "don't use a type caster, declare your function to take an
nb::objectand then convert inside the function body":This is not ideal:
src_objectis a Python sequence.nb::try_castreturns a boolean.nb::castthrows a vague error message.My Solution
My solution involves two parts: a simple wrapper which defers object creation combined with a modified array class which provides detailed error messages.
mpl::defer
mpl::deferis a simple pybind or nanobind wrapper class which defers the creation of an object untilvalue()is called:This is used in a bound function as follows:
mpl::array
mpl::arrayis effectively a vendored version ofnb::ndarrayfrom nanobind's "ndarray.h". It automatically handles Python sequences and provides detailed error messages when a conversion fails. It also allows for "unmanaged" variants which are not backed by a NumPy array.It will eventually replace several of our utility functions and classes:
safe_first_shape()mpl::array::has_members()check_trailing_shape()mpl::shape/mpl::ndimconstraintsclass empty(array.h)mpl::array::unmanaged_empty()class scalar(array.h)mpl::array::unmanaged_scalar()mpl::array::is_same_shape()In this PR, it wraps a
py::arrayand lives in "py_array.h". In a future PR, there will be a variant that wraps annb::ndarrayand lives in "mplarray.h". Both variants can exist as we adopt nanobind.Note
For simplicity, an
mpl::arrayalways has an associated data type and shape. We have a few situations that use data-type-less or shapeless arrays. Those situations will continue to directly usepy::arrayornb::ndarray.Qhull changes
The changes to qhull are straightforward:
py::arraytypes change tompl::array.mutable_data()becomesdata().ndim()check is removed as thempl::arrayconstructor handles this for us.C++ Disclosure
The creation of C++ container classes and heavy template metaprogramming is not something that I do everyday. It involves copious amounts of coffee, epic video game battle music, and intense focus.
Please scrutinize accordingly. While most of the heavy lifting is directly ported from nanobind, I am still filling gaps in my C++ knowledge.
AI Disclosure
PR quality check