add the capsule name to the py::capsule constructor #902
Conversation
tests/test_python_types.cpp
Outdated
| if(ptr){ | ||
| py::print("destructing capsule"); | ||
| } | ||
| })); | ||
| auto name=PyCapsule_GetName(capsule.ptr()); |
There was a problem hiding this comment.
We should provide enough of the interface here that we can avoid hitting the internals, particularly because the test suite also serves as a source of examples. Let's add a simple const char *name() const { return PyCapsule_GetName(m_ptr); } method to get it.
tests/test_python_types.cpp
Outdated
| auto name=PyCapsule_GetName(capsule.ptr()); | ||
| py::print(name); | ||
| auto contents=PyCapsule_GetPointer(capsule.ptr(),name); |
There was a problem hiding this comment.
Here likewise you should be able to use the pybind API: void *contents = capsule;
There was a problem hiding this comment.
Fixed hopefully -- but the pytest is failing for some reason:
> assert capture.unordered == """
created capsule with name --pointer type description-- and contents 1234
creating capsule
destructing capsule
"""
E assert --- actual / +++ expected
E created capsule with name --pointer type description-- and contents 1234
E + creating capsule
E destructing capsule
not sure why it's unhappy.
There was a problem hiding this comment.
It looks like you removed the py::print("creating capsule"); but forgot to take that line out of the expected output in the test.
There was a problem hiding this comment.
ok, I misunderstood the pytest output, added the line back in.
tests/test_python_types.cpp
Outdated
| @@ -575,15 +575,18 @@ test_initializer python_types([](py::module &m) { | |||
|
|
|||
| m.def("return_capsule_with_name_and_destructor_3", | |||
| []() { | |||
| py::print("creating capsule"); | |||
| auto capsule=py::object(py::capsule((void *) 1234, "pointer type description", | |||
There was a problem hiding this comment.
Is there a reason to wrap the capsule inside an object here? capsule is already an object subclass.
|
This looks good to me; any other comments? |
|
Not directly related, but as long as I'm here, a note that my /bin/bash on OSX Sierra won't run tools/check-style.sh $ ./tools/check-style.sh works fine on Ubuntu though. best, Phil |
|
I'll be putting together a Jupyter notebook to demonstrate how to use py::capsule and xtensor-python to pass functions to an ode integrator, will ping you when I post it to my class. |
Thanks for the report; that should be fixed by d080f83 |
|
Merged; thanks for the PR! |
Use `PyCapsule_Destructor` (part of the stable Python ABI) instead of spelling out the C `typedef`. The deprecation message is misleading. Replace with a message pointing to another existing ctor. Background: According to @wjacob the original motivation for deprecating the ctor (in PR pybind#752) was to hide Python C API details, but PR pybind#902 brought those back with a new ctor, it cannot be avoided. Having a `PyCapsule_Destructor` or a `void (*destructor)(void *)` are two separate and valid use cases.
Use `PyCapsule_Destructor` (part of the stable Python ABI) instead of spelling out the C `typedef`. The deprecation message is misleading. Replace with a message pointing to another existing ctor. Background: According to @wjakob the original motivation for deprecating the ctor (in PR pybind#752) was to hide Python C API details, but PR pybind#902 brought those back with a new ctor, it cannot be avoided. Having a `PyCapsule_Destructor` or a `void (*destructor)(void *)` are two separate and valid use cases.
Use `PyCapsule_Destructor` (part of the stable Python ABI) instead of spelling out the C `typedef`. The deprecation message is misleading. Replace with a message pointing to another existing ctor. Background: According to @wjakob the original motivation for deprecating the ctor (in PR #752) was to hide Python C API details, but PR #902 brought those back with a new ctor, it cannot be avoided. Having a `PyCapsule_Destructor` or a `void (*destructor)(void *)` are two separate and valid use cases.
Here's a first attempt at adding a name to py::capsule
I've only changed the first constructor, because trying to add an optional name to the other two constructors produces an error message: