Skip to content

Conversation

@PerretB
Copy link
Contributor

@PerretB PerretB commented Nov 23, 2020

This PR partly solves #130 by adding pybind11 type casters for strided_views and array/tensor_adapters.

As for xarrays and xtensors, those objects are casted to ndarray and do not use any proxy object (such as pyarray or pytensor): I did a bit of refactoring to put all those casters in a dedicated file.

Memory management becomes of course trickier with such objects.  I have the impression that what is done in xtensor_type_caster_base to handle return value policies is generic enough to fit these cases but I am not totally sure. I tried to demonstrate critical cases in the test file (cpp main). The tests validating the correctness of the memory management (keep alive policies) are perhaps not very interesting for regression detection has they focus on pybind11 mechanism rather than xtensor_python and could probably be removed.

@PerretB
Copy link
Contributor Author

PerretB commented Nov 24, 2020

It seems that the CI configurations are outdated:

  • for unix it looks like a python interpreters mismatch
  • for windows, I guess that the latest version of gtest is not compatible with VS 2015

@JohanMabille
Copy link
Member

I'm making a pass on the whole stack to fix all the CI issues (among them, migrating from travis to Azure), it takes time but eventually we'll get there.

@JohanMabille
Copy link
Member

The tests validating the correctness of the memory management (keep alive policies) are perhaps not very interesting for regression detection has they focus on pybind11 mechanism rather than xtensor_python and could probably be removed.

I think we should keep them, this would help detecting any change or regression in pybind11. Even if pybind11 is well tested, having tests specific to our use case is a good thing.

@JohanMabille JohanMabille merged commit a73b40a into xtensor-stack:master Nov 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants