-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow std::complex field with PYBIND11_NUMPY_DTYPE #831
Changes from 1 commit
d383bd8
51b51b6
fb8a4ed
271f380
c8f6488
bd5be69
0556c35
f5818b4
979d979
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -244,7 +244,8 @@ template <typename T> struct is_complex : std::false_type { }; | |
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>, // 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 | ||
std::is_trivially_copyable<T>, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For libstcd++ < 5 (which doesn't have std::is_standard_layout<T>,
#if !defined(__GNUG__) || defined(__clang__) || __GNUC__ >= 5
std::is_trivially_copyable<T>,
#else
std::is_trivially_destructible<T>, std::has_trivial_copy_constructor<T>,
#endif
satisfies_none_of<... It's not quite the same as trivially copyable, but it ought to be close enough in practice. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Closer still:
libstdc++ doesn't appear to have an equivalent |
||
satisfies_none_of<T, std::is_reference, std::is_array, is_std_array, std::is_arithmetic, is_complex, std::is_enum> | ||
>; | ||
|
||
|
@@ -948,7 +949,6 @@ struct field_descriptor { | |
const char *name; | ||
size_t offset; | ||
size_t size; | ||
size_t alignment; | ||
std::string format; | ||
dtype descr; | ||
}; | ||
|
@@ -989,9 +989,10 @@ inline PYBIND11_NOINLINE void register_structured_dtype( | |
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 << '^'; | ||
// mark all fields with '^' (unaligned native type), because numpy | ||
// and C++ don't always agree about alignment (particularly for | ||
// complex, and we're explicitly listing all padding. | ||
oss << '^'; | ||
oss << field.format << ':' << field.name << ':'; | ||
offset = field.offset + field.size; | ||
} | ||
|
@@ -1053,7 +1054,6 @@ template <typename T, typename SFINAE> struct npy_format_descriptor { | |
#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() \ | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have to say, I'm not a big fan of this part. I think it would be cleaner to just remove
FDG
from here and specialize the wholeformat_descriptor
struct forstd::complex
insidecomplex.h
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Especially in light of the extra hack required below for MSVC).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Now for some reason the VS2017 x86 Python 3.6 build (but none of the others) is failing, and there is no indication of what the error is. Also, Travis doesn't seem to have been run at all. Any ideas?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The AppVeyor issue is most likely random: #792. Not sure what happened to Travis.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, looks like it was just random failures. Appveyor is still running the VS2015 builds, but it should all be working now.