gh-60074: add new stable API function PyType_FromMetaModuleAndSpec #93012
Conversation
|
If these changes are acceptable, it would be great if this feature could still be included in Python 3.11. |
That is not possible; we passed the feature freeze with several weeks. You will need to target 3.12. |
|
Isn't that naming a bit off? |
|
@erlend-aasland This means that usable stable API interface for libraries like |
|
@timfel 1�7 (Edit: ultimately converged towards the name |
Ah, right, that was the naming proposal in the old patch. Yes, this does seem pretty long |
You can of course try your case with the 3.11 release manager (Pablo IIRC) :) Quoting PEP-602:
We are now in the 3.11 beta phase. Note that "no new features" are emphasised in bold type. |
|
I would hope @encukou can make a quick call (or give an opinion). The new API probably comes with no compatibility concerns the only thing would be getting the API itself wrong (and even that can be deprecated). For NumPy, it would also be nice to have something like this, but it did not seem particularly pressing (static types work and our DTypes are de-facto immortal anyway). I would still be
But, things don't need to be perfect and besides those two calls to make, I don't think there is any reason to not just do this. We clearly need this problem to be fixed. |
|
I have written a separate email to appeal to @pablogsal. |
|
Could you please add a bunch of tests for this API in |
Yes, I will do this ASAP. Given the valid concern by @timfel that this function sounds like it creates a metatype, I will also change the name of the function to the somewhat long but more descriptive |
|
(I will answer your email as soon as I can) |
Added a new stable API function ``PyType_FromMetaModuleAndSpec``, which mirrors the behavior of ``PyType_FromModuleAndSpec`` except that it takes an additional metaclass argument. This is, e.g., useful for language binding tools that need to store additional information in the type object.
|
@pablogsal - Finished renaming, and test added. |
|
(Could somebody push the button to approve the workflows?) |
|
Please, add a test that checks that we raise if you provide a |
|
@pablogsal done. |
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
@pablogsal I have made the requested changes; please review again. |
|
Thanks for making the requested changes! @pablogsal: please review the changes made to this pull request. |
wjakob
changed the title
|
I would find it odd if this new feature was allowed to be added during beta while we have categorically said no to new features, no matter how small. What's special about this? (Asking @pablogsal, not the OP or other fans of the feature.) |
|
Timeline aside, I'd like to add my support for Project is cppyy, also a C++ - Python binder, but a run-time one, for both CPython and PyPy, used mostly in academia (high energy physics, chemistry, math, biology ...), HPC, and systems. Use of metaclasses is especially important for |
Oh, definitely! I've needed this myself. |
Apparently if I understand correctly this small change is needed by hpy, numpy Devs, pybind11, PyO3 and other binder implementors and I have been told that delaying 1 year the API would make things difficult for all of them (although I still don't fully understand currently what problems this creates in every case). As I mentioned @wjakob by email, feature freeze is quite a strict deadline. All core developers are subjected to it and I mentioned this several times in python-dev and several other forums (including discourse). Additionally, our devguide states the restriction very clearly. Given that this feature seems to have an impact in a lot of packages and the ecosystem as a whole (assuming this is confirmed by the different maintainers) I would like to ask the Steering Council for advice on how to proceed here and if everyone agrees and this is as relevant as I have been told I am willing to consider make an exception. In any case, even if the SC agrees, the maintainers confirm the necessity of this API and there is no other opposition I will not allow this to be added after beta 2, so we have 10 days to collectively make a decision. |
|
Okay, sounds like a plan. We should go full speed ahead with getting the PR merged in main then, so if the SC agrees the backport can be merged right away. |
Misc/NEWS.d/next/Core and Builtins/2022-05-20-13-32-24.gh-issue-15870.e9B-pv.rst
Outdated
Show resolved
Hide resolved
| @@ -3384,6 +3390,12 @@ PyType_FromModuleAndSpec(PyObject *module, PyType_Spec *spec, PyObject *bases) | |||
| char *res_start; | |||
| short slot_offset, subslot_offset; | |||
|
|
|||
| if (meta->tp_new != PyType_Type.tp_new) { | |||
There was a problem hiding this comment.
n.b.: If numpy were to use this to migrate to heap types for the dtypes, their DTypeMeta class currently does define a custom tp_new:
dtypemeta_new(PyTypeObject *NPY_UNUSED(type),
PyObject *NPY_UNUSED(args), PyObject *NPY_UNUSED(kwds))
{
PyErr_SetString(PyExc_TypeError,
"Preliminary-API: Cannot subclass DType.");
return NULL;
}
I'm not sure how important it would be for them to keep this error message, maybe someone from NumPy can comment?
There was a problem hiding this comment.
Disallowing subclassing can already be achieved by clearing the Py_TPFLAGS_BASETYPE flag:
https://docs.python.org/3/c-api/typeobj.html#Py_TPFLAGS_BASETYPE
Lines 3502 to 3508 in 079f0dd
There was a problem hiding this comment.
Yeah, disallowing subclassing might work (of course the dtypes are subclasses, but I guess we don't disallow subclasses in C with that).
In practice, I don't expect NumPy needs a tp_new. I would love to see a dataclass-like API to create a new DType at some point, but that would work differently anyway. So this should be OK.
I would love if @encukou would comment on this, because I think he has a clearer picture than me.
From my current understanding, there were two scenarios:
- This checks makes sense for the current API where a metaclass may exist in the bases! (a current bug that this is ignored, though!). Since we may otherwise forget to initialize the metaclass properly.
- For the new function, it might make sense to tell users that it is their responsibility to make sure the metaclass is properly initialized, and just not add this check at all!
Now, Petr had suggested ApplySpec for the second use-case. I am not sure if that is as convenient in practice (why not call alloc for the user?). But the point is, the new function is for metaclass providers who know what additional initialization is needed for their metaclass (so the check isn't really needed).
|
Adding this functionality would be very useful for Nvidia's Nsight Compute CUDA kernel profiling tool, which relies on the python limited API for its python report interface. The report interface module, which is used by numerous internal and external users to automate access to collected profiling data, is fairly large, and shipping a dedicated module for each potential local python version is not feasible. We therefore currently rely on swig's limited API support, but would like to switch to a more modern alternative such as pybind11, if that had a comparable feature set. As such, I would like to state the support of the Nsight Compute team (and likely of other teams at Nvidia, too), for this change. |
Misc/NEWS.d/next/Core and Builtins/2022-05-20-13-32-24.gh-issue-15870.e9B-pv.rst
Outdated
Show resolved
Hide resolved
|
OT: @wjakob I sent you off-list email. Did you get it? If not can you email me at guido@python.org? |
TL;DR: Numerous flagship Python packages internally rely on
pybind11to bridge C++ and Python. This includes SciPy, PyTorch, Tensorflow, JAX, and many others. This PR adds critical functionality needed to eventually move more of them onto thePy_LIMITED_APIand thereby simplify deployment of binary wheels.Context: C++ <-> Python binding tools like pybind11 and nanobind require the ability to instantiate types using a custom metaclass. pybind11 uses them to install custom handling for some type-related operations, and nanobind goes even further by storing binding-related data structures in an enlarged
PyHeapTypeObjectallocated by a metaclass.Unfortunately is not currently possible to dynamically create suitable heap types using the
Py_LIMITED_API, which means that each extension library using these tools must be compiled many times for redistribution. I believe that it would be useful to the Python community to at least optionally supportPy_LIMITED_APIin these tools, but this requires a small change in CPython.This pull request is another attempt at addressing the issue pointed out in issue #60074. It adds a new stable API function
PyType_FromMetaModuleAndSpecthat mirrors the behavior ofPyType_FromModuleAndSpecexcept that it takes an additional metaclass argument.I used this PR to create a proof-of-concept version of nanobind (🎉 .
limited_apibranch) that dynamically create types using the limited API. It worksI have a somewhat audacious request, which is that this change is considered for Python 3.11 despite having recently entered feature freeze. I believe that this change is small enough and of significant utility for the wider Python community (potentially even for other groups working on bindings like SWIG or Cython)