Allow std::complex field with PYBIND11_NUMPY_DTYPE (#831)

This exposed a few underlying issues:

1. is_pod_struct was too strict to allow this. I've relaxed it to
require only trivially copyable and standard layout, rather than POD
(which additionally requires a trivial constructor, which std::complex
violates).

2. format_descriptor<std::complex<T>>::format() returned numpy format
strings instead of PEP3118 format strings, but register_dtype
feeds format codes of its fields to _dtype_from_pep3118. I've changed it
to return PEP3118 format codes. format_descriptor is a public type, so
this may be considered an incompatible change.

3. register_structured_dtype tried to be smart about whether to mark
fields as unaligned (with ^). However, it's examining the C++ alignment,
rather than what numpy (or possibly PEP3118) thinks the alignment should
be. For complex values those are different. I've made it mark all fields
as ^ unconditionally, which should always be safe even if they are
aligned, because we explicitly mark the padding.
This commit is contained in:
Bruce Merry
2017-05-10 11:36:24 +02:00
committed by Dean Moldovan
parent 8e0d832c7d
commit b82c0f0a2d
6 changed files with 91 additions and 26 deletions

View File

@@ -608,14 +608,14 @@ template <typename T> struct is_fmt_numeric<T, enable_if_t<std::is_arithmetic<T>
};
NAMESPACE_END(detail)
template <typename T> struct format_descriptor<T, detail::enable_if_t<detail::is_fmt_numeric<T>::value>> {
static constexpr const char c = "?bBhHiIqQfdgFDG"[detail::is_fmt_numeric<T>::index];
template <typename T> struct format_descriptor<T, detail::enable_if_t<std::is_arithmetic<T>::value>> {
static constexpr const char c = "?bBhHiIqQfdg"[detail::is_fmt_numeric<T>::index];
static constexpr const char value[2] = { c, '\0' };
static std::string format() { return std::string(1, c); }
};
template <typename T> constexpr const char format_descriptor<
T, detail::enable_if_t<detail::is_fmt_numeric<T>::value>>::value[2];
T, detail::enable_if_t<std::is_arithmetic<T>::value>>::value[2];
/// RAII wrapper that temporarily clears any Python error state
struct error_scope {

View File

@@ -18,10 +18,19 @@
#endif
NAMESPACE_BEGIN(pybind11)
template <typename T> struct format_descriptor<std::complex<T>, detail::enable_if_t<std::is_floating_point<T>::value>> {
static constexpr const char c = format_descriptor<T>::c;
static constexpr const char value[3] = { 'Z', c, '\0' };
static std::string format() { return std::string(value); }
};
template <typename T> constexpr const char format_descriptor<
std::complex<T>, detail::enable_if_t<std::is_floating_point<T>::value>>::value[3];
NAMESPACE_BEGIN(detail)
// The format codes are already in the string in common.h, we just need to provide a specialization
template <typename T> struct is_fmt_numeric<std::complex<T>> {
template <typename T> struct is_fmt_numeric<std::complex<T>, detail::enable_if_t<std::is_floating_point<T>::value>> {
static constexpr bool value = true;
static constexpr int index = is_fmt_numeric<T>::index + 3;
};

View File

@@ -287,7 +287,14 @@ template <typename T, size_t N> struct array_info<T[N]> : array_info<std::array<
template <typename T> using remove_all_extents_t = typename array_info<T>::type;
template <typename T> using is_pod_struct = all_of<
std::is_pod<T>, // since we're accessing directly in memory we need a POD type
std::is_standard_layout<T>, // since we're accessing directly in memory we need a standard layout type
#if !defined(__GNUG__) || defined(__clang__) || __GNUC__ >= 5
std::is_trivially_copyable<T>,
#else
// GCC 4 doesn't implement is_trivially_copyable, so approximate it
std::is_trivially_destructible<T>,
satisfies_any_of<T, std::has_trivial_copy_constructor, std::has_trivial_copy_assign>,
#endif
satisfies_none_of<T, std::is_reference, std::is_array, is_std_array, std::is_arithmetic, is_complex, std::is_enum>
>;
@@ -1016,7 +1023,6 @@ struct field_descriptor {
const char *name;
ssize_t offset;
ssize_t size;
ssize_t alignment;
std::string format;
dtype descr;
};
@@ -1053,13 +1059,15 @@ inline PYBIND11_NOINLINE void register_structured_dtype(
[](const field_descriptor &a, const field_descriptor &b) { return a.offset < b.offset; });
ssize_t offset = 0;
std::ostringstream oss;
oss << "T{";
// mark the structure as unaligned with '^', because numpy and C++ don't
// always agree about alignment (particularly for complex), and we're
// explicitly listing all our padding. This depends on none of the fields
// overriding the endianness. Putting the ^ in front of individual fields
// isn't guaranteed to work due to https://github.com/numpy/numpy/issues/9049
oss << "^T{";
for (auto& field : ordered_fields) {
if (field.offset > offset)
oss << (field.offset - offset) << 'x';
// mark unaligned fields with '^' (unaligned native type)
if (field.offset % field.alignment)
oss << '^';
oss << field.format << ':' << field.name << ':';
offset = field.offset + field.size;
}
@@ -1121,7 +1129,6 @@ private:
#define PYBIND11_FIELD_DESCRIPTOR_EX(T, Field, Name) \
::pybind11::detail::field_descriptor { \
Name, offsetof(T, Field), sizeof(decltype(std::declval<T>().Field)), \
alignof(decltype(std::declval<T>().Field)), \
::pybind11::format_descriptor<decltype(std::declval<T>().Field)>::format(), \
::pybind11::detail::npy_format_descriptor<decltype(std::declval<T>().Field)>::dtype() \
}