Pass *args and **kwargs to superclass in Generic.__new__#517
Conversation
In cases with multiple inheritance, Generic's super may not be object, and so its __new__ requires the same args as __init__.
|
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA). Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. Thanks again to your contribution and we look forward to looking at it! |
| if cls.__origin__ is None: | ||
| return base_cls.__new__(cls) | ||
| if base_cls is object: | ||
| return base_cls.__new__(cls) |
There was a problem hiding this comment.
This special-casing of object worries me a bit. But anyway, this will be necessary only for the backport (with PEP 560 all these hacks will be removed). So if @gvanrossum is fine with this, then it is also OK with me.
There was a problem hiding this comment.
I don't see any viable alternatives. Any suggestions?
There was a problem hiding this comment.
The other alternative is just to keep the status quo. I remember we have a similar situation with GenericMeta.__new__. You can special-case for object, but it looks like that the same can happen with any other class that doesn't expect additional arguments in its __new__.
There was a problem hiding this comment.
You can special-case for object, it looks like that the same can happen with any other class that doesn't expect additional arguments in its new.
The existing code is already a special-case for object: by assuming there are no args in base_cls.__new__ it's assuming base_cls is object. I'm simply adding another case that covers multiple inheritance while preserving the existing special object case.
When using multiple inheritance with Generic and __new__ users will simply have to be responsible and only pass args that are compatible with super's __new__. This is what they would do normally anyway, so it's not asking anything additional of them. Also, note that in my tests I've covered the case where no args are expected.
There was a problem hiding this comment.
What happens if you don't special-case object and always pass *args, **kwds? I find it disturbing that by inheriting directly from object you end up with a class that ignores all arguments passed to it. (Though I realize that there is a problem here when you override one of __init__ and __new__ but not the other.)
There was a problem hiding this comment.
What happens if you don't special-case object and always pass *args, **kwds?
Any class inheriting from Generic whose __init__ takes an arg will fail.
Here's an example of a test case that fails if we pass *args, **kwds to base_cls.__new__:
T = TypeVar('T')
class C(Generic[T]):
def __init__(self, attr):
self.attr = attr
c = C(42)======================================================================
ERROR: test_copy_generic_instances (test_typing.GenericTests)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/Users/chad/dev/typing/src/test_typing.py", line 1113, in test_copy_generic_instances
c = C(42)
File "/Users/chad/dev/typing/src/typing.py", line 1223, in __new__
return _generic_new(cls.__next_in_mro__, cls, *args, **kwds)
File "/Users/chad/dev/typing/src/typing.py", line 1184, in _generic_new
return base_cls.__new__(cls, *args, **kwds)
TypeError: object() takes no parameters
There was a problem hiding this comment.
Though I realize that there is a problem here when you override one of
__init__and__new__but not the other.
I think this is exactly the point. typing.Generic has created this situation for all subclasses, so to avoid inconveniencing users by forcing subclasses with args to implement __new__ and pass the correct args (almost always none), it deals with the problem transparently, by special-casing object. And I'm speaking now about the status-quo, prior to this PR.
There was a problem hiding this comment.
OK, I am beginning to understand the situation. It is indeed a problem (we don't want to require that both __init__ and __new__ process all arguments). But I can defeat your hack by having C subclass a non-generic class D that also override __init__ -- then the __init__ call works fine, but the __new__ call still fails because the superclass isn't object (but still doesn't implement __new__). So I think we should really check whether base_cls.__new__ is object.__new__.
There was a problem hiding this comment.
So I think we should really check whether base_cls.new is object.new.
Deal!
I've signed the CLA and am waiting for the Human on the other end (sorry, Human, I did this twice)
All four cases are hit by the tests (with and without |
OK, thanks! |
|
Fix pushed. |
|
And test added. |
ilevkivskyi
left a comment
There was a problem hiding this comment.
Looks good! I will merge this soon unless Guido wants to wait until after Python 3.6.4 final.
|
I guess since 3.6.4rc1 already went out we shouldn't merge it there, and it
shouldn't be included in the typing 3.6.4 release. So probably best to hold
off here too (unless you've already merged other things that won't go into
3.6.4).
|
|
It looks like master coincides with CPython master now, so I will merge this after typing 3.6.4 is published on PyPI. |
|
New typing was just released, so that we can merge this now. |
|
Hi, when will this change make it onto PyPI? I was under the impression that typing was in sync with Python releases, so I was expecting to see this change in a typing 3.6.5 awhile back. |
|
Hm, we should do a release. By now it should probably be 3.6.6. But I'm not sure when I'll have time, and Ivan's on holiday until the end of July (good for him!). |
I will try to make a release this week (if no other urgent things will appear). |
|
If there are no objections I will release |
|
Hm, I prepared the release but it turns out I don't have rights to upload |
|
Done.
On Sun, Aug 26, 2018 at 9:17 AM Ivan Levkivskyi ***@***.***> wrote:
Hm, I prepared the release but it turns out I don't have rights to upload
typing. I just checked on PyPI <https://pypi.org/project/typing/> and
indeed I am not in the maintainers list. @gvanrossum
<https://github.com/gvanrossum> @ambv <https://github.com/ambv> @JukkaL
<https://github.com/JukkaL> @vlasovskikh <https://github.com/vlasovskikh>
could any of you please add me as a maintainer?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#517 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ACwrMjamjJU-y_kIgnAhH7ugnBm2GeDIks5uUsojgaJpZM4Q8gqG>
.
--
--Guido (mobile)
|
|
Great, thanks! I just uploaded |
In cases with multiple inheritance,
Generic's super may not beobject, and so its__new__requires the same args as__init__.Note that there's special treatment required for the case where the next class in the mro is
object, otherwise every subclass ofGenericthat takes an argument would need to override__new__.