-
Notifications
You must be signed in to change notification settings - Fork 398
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
Make reshape_view accept -1 as a wildcard dimension #2746
Conversation
I got everything working without modifying the unit tests. I question the need to compare if the view and expression have the same shape type. The machinery required to do the cast to std::size_t is pretty complicated for what it is. I'd prefer to drop it. |
I'm probably missing something, but is there not a way to re-use existing machinery? |
I could have reused xcontainer's implementation but the mechanics seemed looked up in a member function. I suppose I could refactor to have a shared implementation, but it doesn't seem right to have the view depend on xcontainer. xtensor/include/xtensor/xcontainer.hpp Line 1118 in 5f49f64
Let me know what you think. |
@JohanMabille @tdegeus I think this one should be implemented as requested. I'd like some suggestions on implementation clean up. I'm still learning my metatemplate syntax and idioms. |
Thanks! Great work. Some minor comments |
Co-authored-by: Tom de Geus <[email protected]>
@JohanMabille I think the PR should be ready to merge! :) |
@spectre-ns thanks a lot for your work on this and your patience! I added some minor comments. |
@JohanMabille I have addressed your comments |
Thanks again! |
Checklist
Description
Fix #2417
@JohanMabille @tdegeus this is failing the type check at test_xstrided_view.cpp(709). I was unable to cast the underlying
std::array
to unsigned usingstd::make_unsigned
as I expected. Any help would be appreciated. I think apart from that it should be identical to the previous implementation as it only implements the check when given signed integers and preserves static lengths usingstd::array<T,N>