-
Notifications
You must be signed in to change notification settings - Fork 160
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
Implement XTensor support in core. #976
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #976 +/- ##
==========================================
- Coverage 86.81% 86.81% -0.01%
==========================================
Files 100 100
Lines 6083 6081 -2
==========================================
- Hits 5281 5279 -2
Misses 802 802 ☔ View full report in Codecov by Sentry. |
include/highfive/xtensor.hpp
Outdated
val.resize(shape); | ||
} |
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.
no recursivity here?
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.
None of it works with non-trivial elements. I should check if they even allow anything non-trivial.
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.
We're now asserting the fact. I propose we revisit, if we ever need to support them.
@alkino The reason for not allowing non-trivial elements, is that this way we can use It doesn't work for anything that contains non-trivial elements. |
7fbae3e
to
a87ef6e
Compare
b24a97d
to
35ab518
Compare
Closes #594. |
8be6326
to
09959cc
Compare
Adds support for `xt::xtensor`, `xt::xarray` and `xt::xview`, both row and column major. This works by wrapping the internal row-major with `xt::adapt`. Therefore, the `T` in `xt::xtensor<T, ...>` must be scalar (trivial).
include/highfive/xtensor.hpp
Outdated
|
||
static hdf5_type* data(type& val) { | ||
if (!is_trivially_copyable) { | ||
throw DataSetException("Invalid used of `inspector<xarray>::data`."); |
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.
not really xarray
here?
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.
Thank you, there used to be separate inspectors for xarray
and xtensor
. Looks like I copied these lines from the xarray
variation.
Adds support for
xt::xtensor
, both row and column major. We're missingxt::xarray
and views. Views fail because they claim to have runtime variable rank.