GeezerVison is showing github.com — PyType_FromSpec should take metaclass as an argument · Issue #60074 · python/cpython
PyType_FromSpec should take metaclass as an argument #60074Description
| BPO |
15870 |
| Nosy |
@loewis, @jcea, @amauryfa, @abalkin, @pitrou, @encukou, @lekma, @mattip, @zooba, @seberg, @ctismer |
| Files |
typeobject.difftypeobject.patch |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = 'https://github.com/abalkin'
closed_at = None
created_at = <Date 2012-09-06.15:49:25.705>
labels = ['interpreter-core', 'type-feature', '3.11']
title = 'PyType_FromSpec should take metaclass as an argument'
updated_at = <Date 2021-10-12.13:28:38.470>
user = 'https://github.com/abalkin' bugs.python.org fields:
activity = <Date 2021-10-12.13:28:38.470>
actor = 'petr.viktorin'
assignee = 'belopolsky'
closed = False
closed_date = None
closer = None
components = ['Interpreter Core']
creation = <Date 2012-09-06.15:49:25.705>
creator = 'belopolsky'
dependencies = []
files = ['27137', '50285']
hgrepos = []
issue_num = 15870
keywords = ['patch', 'needs review']
message_count = 40.0
messages = ['169925', '169928', '169929', '169940', '169942', '169943', '169944', '169951', '169955', '169972', '169977', '398430', '398435', '398751', '398776', '401642', '401651', '401781', '402105', '402511', '402517', '402520', '402521', '402522', '402527', '402551', '402729', '402733', '402734', '402735', '402737', '402738', '402751', '402753', '402793', '402800', '402806', '403231', '403257', '403730']
nosy_count = 16.0
nosy_names = ['loewis', 'jcea', 'amaury.forgeotdarc', 'belopolsky', 'pitrou', 'Arfrever', 'petr.viktorin', 'lekma', 'Alexander.Belopolsky', 'mattip', 'Robin.Schreiber', 'steve.dower', 'seberg', 'Christian.Tismer', 'jhaberman', 'haberman2']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue15870'
versions = ['Python 3.11'] Activityabalkin commented on Sep 6, 2012 PyType_FromSpec() is a convenient function to create types dynamically in C extension modules, but its usefulness is limited by the fact that it creates new types using the default metaclass.
I suggest adding a new C API function
PyObject *PyType_FromSpecEx(PyObject *meta, PyType_Spec *spec)
and redefine PyType_FromSpec() as
PyType_FromSpecEx((PyObject *)&PyType_Type, spec)
This functionality cannot be implemented by user because PyType_FromSpec requires access to private slotoffsets table.
A (trivial) patch attached. amauryfa commented on Sep 6, 2012 The patch is a bit light: see how type_new also computes the metaclass from the base classes. AlexanderBelopolsky commented on Sep 6, 2012 AlexanderBelopolskymannequin On Thu, Sep 6, 2012 at 12:44 PM, Amaury Forgeot d'Arc
<[email protected]> wrote:
The patch is a bit light: see how type_new also computes the metaclass from the base classes.
This was intentional. I was looking for a lightweight facility to
create heap types. I know metaclass when I call PyType_FromSpec. If
i wanted to invoke the "metaclass from the base classes" logic, I
would just specify an appropriate base class in the spec. This would
still leave an open problem of specifying the metatype for the most
basic class. This is the problem I am trying to solve. abalkin commented on Sep 6, 2012
see how type_new also computes the metaclass from the base classes.
As you can see from my first message, I originally considered PyType_FromSpecEx(PyObject *meta, PyType_Spec *spec) without bases. (In fact I was unaware of the recent addition of PyType_FromSpecWithBases.) Maybe the original signature makes more sense than the one in the patch. Explicitly setting a metaclass is most useful for the most basic type. On the other hand, a fully general function may eventually replace both PyType_FromSpec and PyType_FromSpecWithBases for most uses. loewis commented on Sep 6, 2012 What is your use case for this API? AlexanderBelopolsky commented on Sep 6, 2012 AlexanderBelopolskymannequin On Sep 6, 2012, at 5:10 PM, Martin v. Löwis <[email protected]> wrote:
What is your use case for this API?
I can describe my use case, but it is somewhat similar to ctypes. I searched the tracker for a PEP-3121 refactoring applied to ctypes and could not find any. I'll try to come up with a PEP-3121 patch for ctypes using the proposed API. loewis commented on Sep 6, 2012 If it's very special, I'm -0 on this addition. This sounds like this is something very few people would ever need, and they can learn to write more complicated code to achieve the same effect. Convenience API exists to make the common case convenient.
I'm -1 on calling it PyType_FromSpecEx. AlexanderBelopolsky commented on Sep 6, 2012 AlexanderBelopolskymannequin On Sep 6, 2012, at 6:25 PM, Martin v. Löwis <[email protected]> wrote:
I'm -1 on calling it PyType_FromSpecEx.
I find it encouraging that you commented on the choice of name. :-) I can live with PyType_FromMetatypeAndSpec and leave out bases. PyType_FromTypeAndSpec is fine too.
On the substance, I don't think this API is just convenience. In my application I have to replace meta type after my type is created with PyType_FromSpec. This is fragile and works only for very simple metatypes.
Let's get back to this discussion once I have a ctypes patch. I there will be a work-around for ctypes it will probably work for my case. (My case is a little bit more complicated because I extend the size of my type objects to store custom metadata. Ctypes fudge this issue by hiding extra data in a custom tp_dict. ) pitrou commented on Sep 6, 2012 This API may make it easier to declare ABCs in C. loewis commented on Sep 7, 2012 As for declaring ABCs: I don't think the API is necessary, or even helps. An ABC is best created by *calling* ABCMeta, with the appropriate name, a possibly-empty bases tuple, and a dict. What FromSpec could do is to fill out slots with custom functions, which won't be necessary or desirable for ABCs. The really tedious part may be to put all the abstract methods into the ABC, for which having a TypeSpec doesn't help at all. (But I would certainly agree that simplifying creation of ABCs in extension modules is a worthwhile reason for an API addition)
For the case that Alexander apparently envisions, i.e. metaclasses where the resulting type objects extend the layout of heap types: it should be possible for an extension module to fill out the entire type "from scratch". This will require knowledge of the layout of heap types, so it can't use just the stable ABI - however, doing this through the stable ABI won't be possible, anyway, since the extended layout needs to know how large a HeapType structure is.
If filling out a type with all slots one-by-one is considered too tedious, and patching ob_type too hacky - here is another approach: Use FromSpec to create a type with all slots filled out, then call the metatype to create a subtype of that. I.e. the type which is based on a metatype would actually be a derived class of the type which has the slots defined. pitrou commented on Sep 7, 2012
If filling out a type with all slots one-by-one is considered too
tedious, and patching ob_type too hacky - here is another approach:
Use FromSpec to create a type with all slots filled out, then call the
metatype to create a subtype of that. I.e. the type which is based on
a metatype would actually be a derived class of the type which has the
slots defined.
As a matter of fact, this is what the io module is doing (except that
the derived type is written in Python). It does feel like pointless
complication, though. mattip commented on Feb 14, 2023 here is the work-around in nanobind for versions before 3.12 ctismer commented on Feb 14, 2023 @mattip Versions before 3.12 are no problem. And whatever I do gets redirected to PyType_FromMetaclass in 3.12, which then forbids tp_new.
I was never stuck, but now I am. Why is tp_new changing not allowed? seberg commented on Feb 14, 2023 There was more discussion on the newer PR I think, but also here: gh-89546. The issue is that the super().__new__() cannot be called and Petr at that time suggested a second function may be necessary, but I think later it was thought/hoped that just wouldn't be needed. Reacted by Erlend E. Aasland Reacted by Christian Tismer-Sperling and yopito ctismer commented on Feb 15, 2023 @seberg Thanks a lot, now I understand the problem better. And as I read all the discussions, the tp_new problem was not solved in the end, although there was a plan.
On first glance I was happy that this hackery around PyType_* would come to an end, but with the redirections leading always into PyType_FromMetaclass, there is effectively a new restriction that was not present before.
I need to test if some hacking aftter the fact can work around that. encukou commented on Feb 15, 2023 If there is a tp_new, what should PyType_FromMetaclass (or any type creation function that doesn't take *args, **kwargs) do? ctismer commented on Feb 15, 2023 It should simply do nothing but accept it as-is.
Before the new function existed, we supplied the metatype after type crestion.
That should simply work as before. We do not supply the metatype to PyType_From*, but some helper function now grabs the likely metatype from the bases, adds it and then complains about our tp_ new.
We don‘t have a chance to patch it ourselves any more.
I don‘t complain about the new function, this may be ok for the use cases. But it should not be greedy and leave my patching alone. This is not an addition to the API but it removes functionality without replacement. seberg commented on Feb 15, 2023 Before the new function existed PySide worked by pure luck because Python always allocated one slot extra which happened to be enough for its use-case. (It worked for PySide, but not really anyone else, took me weeks to fully understand probably at the time.)
In NumPy I currently also have a tp_new, all it does is reject creation, though. IIRC there was a point that it should be OK to do without/remove (I don't remember the actual point, but trusted it at the time). (NumPy's hacks are deep enough they should keep working in 3.12 for the time being.)
So, I agree we need to find a solution. But we should also ask the question if you actually need the tp_new, and whether the right approach is not to allow it but to add a way to write a "C new" function for metaclasses. Reacted by Erlend E. Aasland, Christian Tismer-Sperling and Neil Girdhar encukou commented on Feb 15, 2023
It should simply do nothing but accept it as-is.
That means it would create an object without calling its C-level initialization function. That sounds pretty dangerous -- most tp_new set up some invariant you should then be able to rely on.
I don't understand the use case here. Why do you have a tp_new that should not be called at instance creation?
add a way to write a "C new" function for metaclasses
But does that “C new” need to be a slot? It probably needs a custom signature -- if you can fit in *args, **kwargs you should be able to use tp_new, and call the metatype to create subclasses. And if that's the case, there's not much point in letting Python or other extensions know about it -- it can just be a C function you export. Reacted by Erlend E. Aasland and Christian Tismer-Sperling seberg commented on Feb 15, 2023
But does that “C new” need to be a slot?
No, of course not. In the old issue you had brought up a PyType_ApplySpec as a raw interface to creating a new class instance. Having such a function would then allow me in NumPy for example to have a C-API function:
DTypeMeta
PyArray_CreateNewDTypeFromSpec(DTypeMeta_Spec *spec)
{
newtype = alloc_metatype();
PyType_ApplySpec(newtype, some_spec);
/* Do more initialization stuff */
return newtype;
}
Now, I am not convinced we need this, because all I currently need in NumPy is rejecting creation in Python. And if I would want more, I would just create a factory function anyway, probably:
@create_dtype
class SomeClass:
...
which is more than enough probably. encukou commented on Feb 15, 2023
all I currently need in NumPy is rejecting creation in Python
Is doing that in tp_init not enough?
I guess there is a way to bypass tp_init from Python...
In the old issue you had brought up a PyType_ApplySpec as a raw interface to creating a new class instance.
A good first step would be PyType_SetSlot, initially with a small subset of settable slots (for some slots, setting them after creation is tricky). (BTW, that would bring us closer to Mark's sketch of a perfect API, while keeping current C signatures.) Reacted by Sebastian Berg, Erlend E. Aasland and Christian Tismer-Sperling ctismer commented on Feb 15, 2023 @seberg
So, I agree we need to find a solution. But we should also ask the question if you actually need the tp_new, and whether the right approach is not to allow it but to add a way to write a "C new" function for metaclasses.
PySide has a meta type Shiboken.ObjectType which is used in all PySide classes.
When a wrapper type is initialized, its type is created this way.
This code existed long before my time in PySide1. When I moved to Qt5 (2015) and then limited API (2017),
the trouble began. 😉
Basically, we add some extra attributes to the meta type. But in 2021, supporting PyPy forced me to
put these extras into a shadow structure, anyway. So it might be possible to add them later when the type
creation is over. I'm currently investigating that.
This is done. The much simpler solution is below: ctismer commented on Feb 15, 2023 @seberg I have a crude fix:
Before calling PyType_FromSpecWithBases or PyType_FromMetaclass, I patch an eventually
existing tp_new away and restore it right afterwards. This gives the same behavior as before.
It does not hurt (but the brain) , because only the type slot is temporarily changed, but the __new__
function is still in the already initialized meta type.
As soon as a proper "C new" function is available, I will remove that hack. But it serves me well,
for heaven's sake. You could do the same BTW. to your NumPy tp_new.
Hoping for a wise decision on this weird issue 😄
So long -- Chris Reacted by Gregory P. Smith encukou commented on Mar 22, 2023 Sorry for dropping the ball here.
The behavior change is backwards incompatible. If refusing to create classes with __new__ is better than silently skipping it (as I think it is), there should at least be a deprecation warning period, at least in the pre-existing functions.
I'll do that for 3.12.
I assume this won't really help you since PySide will want to avoid deprecation warnings, though...
As hacky as it is, patching tp_new away at least says I don't want tp_new called and I know what I'm doing explicitly. Reacted by Gregory P. Smith ctismer commented on Apr 27, 2023 @encukou Whatever the outcome, it's fine with me. The most important thing is a stable status that will not change for the time being.
All the best -- Chris encukou commented on Jun 5, 2023
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|