GeezerVison is showing github.com — PyType_FromSpec refuses to create classes with tp_new · Issue #103968 · python/cpython
PyType_FromSpec refuses to create classes with tp_new #103968DescriptionAs reported in #60074, since PyType_FromMetaclass was added, other functions from the PyType_FromSpec family refuse to create classes whose metaclass has a non-default tp_new. IMO this is the correct default behaviour -- we don't have the arguments to call tp_new with, and skipping it may be dangerous.
Nevertheless, it's a backwards-incompatible change. It should only be made after a deprecation period.
I'm working on a fix.
Linked PRs
Activityadded 2 commits that reference this issue on Apr 28, 2023gh-103968: Deprecate creating heap types whose metaclass has custom t… encukou commented on May 3, 2023 Deprecation period started. Reacted by Erlend E. Aasland gpshead commented on Jun 2, 2023 The 3.12 docs now say "The tp_new of the metaclass is ignored. which may result in incomplete initialization." statements, but it isn't clear if this is a part of the versionchanged in 3.12 behavior, or if this was always the case.
Basically, we need explicit instructions on what code finding itself in the situation that protobuf protocolbuffers/protobuf#12186 finds itself in is supposed to do to reconcile the situation.
3.12's "What's New" does not directly address this today. It just talks about tp_new being ignored as if that is a behavior change rather than if it was always ignored without being documented as such or what should be done to get the previous behavior. Reacted by C.A.M. Gerlach gpshead commented on Jun 2, 2023 Late discussion on #60074 which created the TypeError that this issue properly made into a DeprecationWarning has suggestions for a workaround but they definitely qualify as hacks. Deeper understanding of why that tp_new was set and what it's goal was and when it was ever called or not in the past is likely required. haberman commented on Jun 13, 2023
protobuf uses PyType_FromSpecWithBases to create ScalarMapContainer and MessageMapContainer, which derive from collections.abc.MutableMapping, so their metaclass is abc.ABCMeta.
Protobuf is using MutableMapping as a mixin. The goal is that we can define only a core set of methods (__getitem__, __setitem__, etc.) and get correct implementations of all other MutableMapping methods "for free" (__contains__, keys, etc.).
abc.ABCMeta defines __new__, meaning it overrides how ABC types should be constructed.
This __new__ is not called on protobuf's container types, which is, IMO, a bug.
It looks like the primary purpose of abc.ABCMeta is to add a register() method to the class, so that you can call MutableMapping.register(Foo), which will cause isinstance(Foo(), MutableMapping) to return true.
We do not want that functionality in this case. We do not want people to be able to call ScalarMapContainer.register(Foo), because ScalarMapContainer is a concrete class, not an abstract class.
I suspect that anyone who uses MutableMapping as a mixin is in the same boat as us. I would consider it a bug if a class becomes an ABC merely because it mixed in an ABC.
But this seems to be what happens if you mix in an ABC in pure Python:
from collections.abc import MutableMapping
class MyMapping(MutableMapping):
pass
class OtherMapping:
pass
# This works -- MyMapping has become an ABC!
MyMapping.register(OtherMapping)
assert isinstance(OtherMapping(), MyMapping)The simplest fix would be to stop inheriting from MutableMapping. Maybe we could "manually" mix in the other methods without using inheritance:
from collections.abc import MutableMapping
class ScalarMapContainer:
# ...
MutableMapping.register(ScalarMapContainer)
# Mixin the methods manually.
ScalarMapContainer.__contains__ = MutableMapping.__contains__
ScalarMapContainer.keys = MutableMapping.keys
ScalarMapContainer.items = MutableMapping.items
ScalarMapContainer.values = MutableMapping.values
# ...Reacted by Gregory P. Smith and David Lechner Yhg1s commented on Jul 11, 2023 Is there anything left to do here for 3.12? encukou commented on Jul 11, 2023 No, short of reverting the warning. added a commit that references this issue on Jul 11, 2023gh-103968: What's New: Add porting hints for PyType_From with metacla… cdce8p commented on Aug 6, 2023 Just saw the warning while testing 3.12.0b4. Is stack_level=1 correct for the DeprecationWarning
This is my test output
<frozen importlib._bootstrap>:400
<frozen importlib._bootstrap>:400
<frozen importlib._bootstrap>:400: DeprecationWarning: Using PyType_Spec with metaclasses that have custom tp_new is deprecated and will no longer be allowed in Python 3.14.
After reading the discussion here, it could probably be Protobuf but no way to tell for the warning alone.
Happy to open a new issue if this isn't the right place. gpshead commented on Aug 7, 2023
After reading the discussion here, it could probably be Protobuf but no way to tell for the warning alone.
Happy to open a new issue if this isn't the right place.
@cdce8p - If you can reproduce this on 3.12.0rc1 please open a new issue with reproducer details (and ideally a link to the protobuf code in question). osandov commented on Nov 13, 2024 Leaving a note here for others that may run into this issue with collections.abc. Another workaround is to define your desired extension class as a mixin type and then create the final type by inheriting from both your mixin type and the abstract base class. CPython's own decimal module does this:
|
/* Create SignalDict type */ |
|
ASSIGN_PTR(state->PyDecSignalDict_Type, |
|
(PyTypeObject *)PyObject_CallFunction( |
|
(PyObject *)&PyType_Type, "s(OO){}", |
|
"SignalDict", state->PyDecSignalDictMixin_Type, |
|
MutableMapping)); |
. haberman commented on Nov 18, 2024
MetadataMetadataStatusShow more project fields
DevelopmentNo branches or pull requestsIssue actions
|