Uh oh!
There was an error while loading. Please reload this page.
Numpy: better compilation errors, long double support - #619
Conversation
The current documentation and example reads as though PYBIND11_NUMPY_DTYPE is a declarative macro along the same lines as PYBIND11_DECLARE_HOLDER_TYPE, but it isn't. The changes the documentation and docs example to make it clear that you need to "call" the macro.
aldanor
commented
Jan 26, 2017
Looks all good to me. Just wondering, why the |
jagerman
commented
Jan 26, 2017
Two reasons: first |
aldanor
commented
Jan 26, 2017
Oh, I see, good then. Can't remember to have encountered |
| template <class T, template<class> class... Predicates> using is_all_of = all_of<Predicates<T>...>; | ||
| template <class T, template<class> class... Predicates> using is_any_of = any_of<Predicates<T>...>; | ||
| template <class T, template<class> class... Predicates> using is_none_of = none_of<Predicates<T>...>; |
There was a problem hiding this comment.
Having all_of and is_all_of might be a little confusing. Maybe satisfies_all_of?
| template <typename T> using is_pod_struct = all_of< | ||
| std::is_pod<T>, // offsetof only works correctly for POD types | ||
| is_none_of<T, std::is_reference, std::is_array, is_std_array, std::is_arithmetic, is_complex, std::is_enum> |
There was a problem hiding this comment.
Why is specifically std::array disallowed?
There was a problem hiding this comment.
Because it was before. (I didn't change the logic here, just cleaned it up a bit with convenience meta-templates). I seem to recall some discussion about coming back to handle that eventually (cc @aldanor?) at some point post 2.0.
| template <typename T> struct is_complex<std::complex<T>> : std::true_type { }; | ||
| template <typename T> using is_pod_struct = all_of< | ||
| std::is_pod<T>, // offsetof only works correctly for POD types |
There was a problem hiding this comment.
Nitpicking: offsetof only needs is_standard_layout, it's numpy that needs the rest of is_pod.
There was a problem hiding this comment.
I just copied the comment, but I'll update it.
`satisfies_all_of<T, Pred1, Pred2, Pred3>` is a nice legibility-enhanced shortcut for `is_all<Pred1<T>, Pred2<T>, Pred3<T>>`.
If you try to use a non-POD data type, you get difficult-to-interpret compilation errors (about ::name() not being a member of an internal pybind11 struct, among others), for which isn't at all obvious what the problem is. This adds a static_assert for such cases. It also changes the base case from an empty struct to the is_pod_struct case by no longer using `enable_if<is_pod_struct>` but instead using a static_assert: thus specializations avoid the base class, POD types work, and non-POD types (and unimplemented POD types like std::array) get a more informative static_assert failure.
numpy.h uses unprefixed macros, which seems undesirable. This prefixes them with PYBIND11_ to match all the other macros in numpy.h (and elsewhere).
5b7d3ff to
c571b78Comparejagerman
commented
Jan 27, 2017
Not sure how I accidentally undid the DECL_NPY_API ones, but I fixed that too (and squashed everything). |
This adds long double and std::complex<long double> support for numpy arrays. This allows some simplification of the code used to generate format descriptors; the new code uses fewer macros, instead putting the code as different templated options; the template conditions end up simpler with this because we are now supporting all basic C++ arithmetic types (and so can use is_arithmetic instead of is_integral + multiple different specializations). In addition to testing that it is indeed working in the test script, it also adds various offset and size calculations there, which fixes the test failures under x86 compilations.
c571b78 to
3fc4f7dComparewjakob
commented
Jan 31, 2017
Looks all good, thank you! |
A few small-ish changes related to numpy:
long double(andstd::complex<long double>) support: there's not a huge reason to not support them--supporting all floating point types makes the code a little cleaner--and this allows binding of Eigen code that has long doubles.The changes to the numpy dtype test script also resolve the linux/i386 test failure from #612.