Adding possibility to 'cast' or copy to xt::xarray etc
#267
Conversation
xt::xarray etc
There was a problem hiding this comment.
I'm not sure it would make sense to implement the cast operator for strided_views (which is used for creating a view on an existing expression).
It would make sense to implement it for the xarray_adaptor, though. It would be a non-owning adpator on the underlying buffer.
| template <class T> | ||
| struct xtensor_check_buffer | ||
| { | ||
| }; |
There was a problem hiding this comment.
I think you can omit the definition of the generic case. A simple declaratoin should be enough.
There was a problem hiding this comment.
I thought so too, but my compiler complained:
xtensor_type_caster_base.hpp:50:16: error: no template named 'xtensor_check_buffer'; did you mean 'xtensor_get_buffer'?
struct xtensor_check_buffer<xt::xarray<T, L>>
^~~~~~~~~~~~~~~~~~~~
xtensor_get_buffer| std::vector<size_t> shape(buf.ndim()); | ||
| std::copy(buf.shape(), buf.shape() + buf.ndim(), shape.begin()); | ||
| value = Type(shape); | ||
| std::copy(buf.data(), buf.data() + buf.size(), value.begin()); |
There was a problem hiding this comment.
I think we should take into consideration the strides of the numpy array. There is no guarantee that the element of the arrayare contiguous in memory (the array could actually be a view).
The strides number are numbers of bytes instead of numbers of elements, so we have to handle the conversion. Or we can use the pystride adaptor class.
There was a problem hiding this comment.
Ah, I did not know about the pystride adaptor. So for the moment I use pybind11's way to ensure that data is contiguous, by using https://pybind11.readthedocs.io/en/stable/advanced/pycpp/numpy.html#arrays. Personally I have no big feelings about one or the other, but at the same time I am not entirely sure about the difference in efficiency.
There was a problem hiding this comment.
Ah indeed c_style and f_style involve contiguity, I never paid attention to that, thanks for the pointer!
|
Essentially this should be good to go. However, I had to introduce on additional check to check on rank for static auto get(pybind11::handle src)
{
auto buf = pybind11::array_t<T, pybind11::array::c_style | pybind11::array::forcecast>::ensure(src);
if (buf.ndim() != 2) {
return false;
}
return buf;
}however my compiler gets (rightfully) confused: So I worked around with the extra check. I would like to get rid of it. But I don't know what I can return instead of if (!buf) {
return false;
}Alternatively on could try to get rid of the check statically. But this I don't know how to do easily. |
| { | ||
| return array_t<T, array::c_style | array::forcecast>::ensure(src); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Nitpicking: what about the following renaming?
xtensor_get_buffer => pybind_array_getter_impl
get => run
| { | ||
| return xtensor_get_buffer<T, L>::get(src); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Nitpicking: in line with the previous renaming proposal:
xtensor_chek_buffer => pybind_array_getter
get => run
| { | ||
| return true; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Nitpicking:
xtensor_verify => pybind_array_dim_checker
get => run
| { | ||
| using T = typename Type::value_type; | ||
|
|
||
| if (!convert && !array_t<T>::check_(src)) { |
There was a problem hiding this comment.
Nitpicking: opening curly brace goes on a new line.
|
Thanks @JohanMabille ! Good to go. |
No description provided.