-
Notifications
You must be signed in to change notification settings - Fork 20
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
Add __dict__ to LowLevelView #7
Comments
I tried compiling my own libcppyy.so and using it as a drop-in replacement, but couldn't get that to work. Depending on the commit I used I either got a segfault or a dlopen ImportError (symbol not found in flat namespace '__ZN5Cppyy9ConstructEmPv'). As such, I wasn't able to test this patch. So treat it as a starting point rather than a complete patch. |
Note that a big change in 3.0.0 is the move to Clang13 and the use of C++20. Separately, |
So how would I put a lifeline on the vector in practice? Also, if you have a hard ref from the vector to the LowLevelView object, then what I'm proposing won't work because the LowLevelView object's refcount never reaches 0 and so it'll never trigger the weakref callback, meaning all my vectors would leak because they'd be stored in that dictionary forever. |
currently can't, because the view has no place to set it. It would need a new data member.
The other way around, in pseudo-code: |
I guess I tried that at some point and explored the weakref approach because I couldn't set any attributes on LowLevelView. But if we're making changes to the C++ implementation of LowLevelView, then I agree with you it's probably best to just make it support a |
If I understand the documentation right, attached patch should be all that's needed to add a I tried compiling the various cppyy packages from source in order to test this, but I'm having trouble compiling cling (see below for details). @wlav would you consider adding a change like this? Error compiling cling:
|
EDIT: originally this was a feature request to add weak reference support to the LowLevelView type. After discussion it seems like adding support for a
__dict__
is a better solution.I have a specific use case for this I will explain below. However I'm open to suggestions for alternative solutions. I looked into using special variables and couldn't come up with a solution, though I must admit I'm not very confident about my understanding of what these variables do.
Suppose I have C++ functions that return
std::vector
. The goal is to turn these into numpy arrays without copying any data. The challenge is I want to be able to keep the underlying C++ objects alive until no python object is referencing the LowLevelView anymore.To illustrate the issue, consider this code:
Output:
We know we need to keep the C++ vector alive for as long as anything is referencing the LowLevelView object pointing to
vec.data()
. One way to do this is to wrap the LowLevelView object with a small wrapper class that also stores a reference to vec and make surearr.base
points to an instance of this class. The issue is this wrapper class needs to "forward" the buffer protocol, i.e. it needs to present the buffer protocol from its LowLevelView member to its callers. I don't think this is currently possible. PEP688 may change this starting in 3.12.Another way to achieve the same behavior would be to keep a reference to vec until the last reference to the LowLevelView goes out of scope. To that end, we could keep a dictionary where the keys are weak references to LowLevelViews and the values are the corresponding std::vectors. Continuing from the previous code:
I think this would do the trick, but it requires LowLevelView to support weak referencing.
The text was updated successfully, but these errors were encountered: