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
object.__new__ argument calling autodetection faulty #49572
Comments
In 2.6 a deprecation warning was added if The following code shows the problem: >>> class A(object):
... def __new__(self):
... raise TypeError('i do not exist')
...
>>> class B(A):
... __new__ = object.__new__
... def __init__(self, x):
... self.x = x
...
>>> B(1)
__main__:1: DeprecationWarning: object.__new__() takes no parameters
<__main__.B object at 0x88dd0> In the I used the pattern with the "__new__ switch" to achieve a Real-world use case here: |
The problem seems to be caused by tp_new being slot_tp_new which then I'm not so sure what would be the solution to this. One could of course |
I think the real problem here is For example the following snippet: >>> class Dict(dict): __new__ = object.__new__
...
>>> Dict.__new__ is object.__new__
True
>>> Dict()
{} I would rather expect this behaviour (or at least that Dict.__new__ is not >>> Dict.__new__ is object.__new__
True
>>> Dict()
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: object.__new__(Dict) is not safe, use dict.__new__() The attached patch leads to that behaviour, which also fixes the argument |
I'm sorry, I don't have any opinion on this. |
The problem is that The affected code in |
See also issue bpo-1694663. |
I think it needs tests. |
Here are updated patches with tests for 3.x and 2.7. |
New changeset a37cc3d926ec by Serhiy Storchaka in branch '2.7': |
Will commit to 3.5-3.7 after releasing 3.6.0. |
New changeset 1f31bf3f76f5 by Serhiy Storchaka in branch '3.5': New changeset 747de8acb7e4 by Serhiy Storchaka in branch '3.6': New changeset 9605c558ab58 by Serhiy Storchaka in branch 'default': |
test_file started to crash after the change "Issue bpo-5322: Fixed setting __new__ to a PyCFunction inside Python code." :-/ (so all buildbots became red.) Can someone fix it or revert it? (Sorry, I don't have the bandwith right to investigate the crash.) |
The test is fixed if change order of base classes of UnsupportedOperation. This is rather a workaround, we should find more general fix. |
New changeset 4a610bc8577b by Serhiy Storchaka in branch '3.5': |
This change breaks backward compatibility in Python 2.7. This is the example that also broke in bpo-25731. In that case the change was reverted. See https://bugs.python.org/issue25731#msg262922 $ cat foo.pxd
cdef class B:
cdef object b
$ cat foo.pyx
cdef class A:
pass cdef class B: class C(A, B):
def __init__(self):
B.__init__(self, 1)
C()
$ cython foo.pyx && gcc -I/usr/include/python2.7 -Wall -shared -fPIC -o foo.so foo.c
$ python -c 'import bar'
Traceback (most recent call last):
File "<string>", line 1, in <module>
File "bar.py", line 7, in <module>
C()
TypeError: foo.A.__new__(C) is not safe, use foo.B.__new__() |
Here is more minimal breaking example. This clearly shows that this patch breaks backwards compatibility.
|
Does changing the order of base classes help or there is an unavoidable conflict? |
https://trac.sagemath.org/ticket/22037 reports about another regression. |
@serhiy.storchaka: yes, changing the order of the base classes fixes the issue with __new__. Also manually assigning __new__ works, like class C(A, B):
__new__ = B.__new__ What is broken by this patch is only the auto-detection of which __new__ (really, which tp_new) should be called. @Doko: not "another regression", it's exactly the one that we are talking about. |
Thank you Jeroen. It looks to me that all problems can be resolved by reordering base classes and making Cython not generating trivial __new__. But that is possible only in new Python version. In maintained versions we should keep the old behavior for backward compatibility even if it contradicts normal rules for method resolution and the behavior of Python classes. We should find other solution for making explicit __new__ assigning working. |
Wouldn't it be possible to fix assignment of __new__ without breaking backwards compatibility (and then apply the same patch for all Python versions)? I have a feeling that breaking the auto-detection of tp_new is a new bug introduced by this patch and not a fundamental feature needed to fix assignment of __new__. |
New changeset 5315db3171b0 by Benjamin Peterson in branch '2.7': |
Since this change seems to break the backward compatibility, is it safe to apply it to Python 3.5.x and Python 3.6.x? The bug was reported in 2009, 7 years ago. Can the fix wait for Python 3.7? test_file contains code which worked well before the change and started to crash after the change. If it occurs for an application, I expect users to be unhappy of getting such "behaviour change" in a minor release, no? -- Is it possible to prevent the crash of test_file without modifying its code (without the change 4a610bc8577b "Change order of io.UnsupportedOperation base classes")? Sorry, I didnd't follow this issue. |
New changeset f89ef18f9824 by Serhiy Storchaka in branch '2.7': New changeset 06e4b9f2e4b0 by Serhiy Storchaka in branch '3.5': |
BTW, at least for bpo-25731, I think the right approach in the MI case is to synthesize a __new__ on the subclass that calls the solid base __new__. |
Yes, it was what the patch did by setting tp_new to slot_tp_new. The problem is that the same code is used for inherited __new__ and assigned in class body. It is hard to distinguish between these cases. In any case I think that Cython shouldn't generate trivial __new__. This will help to change the order of __new__ resolution at least in 3.7. |
It worries me that nothing in the Python docs nor in any PEP describes how tp_new is inherited. In my opinion, this patch makes a significant change which should be subject to a PEP. However, neither the old nor new behaviour is described anywhere. This also makes it harder to argue which behaviour is correct. |
Can we close this now? >>> class A(object):
... def __new__(self):
... raise TypeError('i do not exist')
...
>>> class B(A):
... __new__ = object.__new__
... def __init__(self, x):
... self.x = x
...
>>> B(1)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: object.__new__() takes exactly one argument (the type to instantiate)
>>> |
The bug is still there, just that it's now not just a warning but an error. The auto detection is incorrect here. It should allow the instantiation of the object with arguments. |
Right, I see now. |
Python has the same behavior since Python 2.6. While it annoys a few persons, the majority doesn't care. I suggest to close the issue. It's easy to workaround limitation that object.__new__() which doesn't accept arguments. |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: