Please revert f4be03b3 (libvirtaio: Drop object(*args, **kwargs)) for theoretical reasons

[Daniel P. Berrangé]

On Wed, Aug 19, 2020 at 11:32:10PM +0200, Wojtek Porczyk wrote:
> Hi Philipp,
> (Cc: Daniel, because IIUC you reviewed !16 which got this merged),
> 
> I'm sorry I didn't notice this earlier, but the commit f4be03b3 dated
> 2020-04-20 [0] is wrong. The super().__init__(*args, **kwargs) in
> Callback.__init__ was there on purpose, because of how Python's inheritance in
> new-style classes works.
> 
> Let me explain this a bit, because it is not obvious.
> 
> Suppose you had diamond inheritance like this:
> 
>     class A(object): pass
>     class B(A): pass
>     class C(A): pass
>     class D(B,C): pass
> 
> And those classes needed a common function with varying arguments:
> 
>     class A(object):
>         def spam(self, a): print(f'A: {a}')
>     class B(A):
>         def spam(self, b): print(f'B: {b}')
>     class C(A):
>         def spam(self, c): print(f'C: {c}')
>     class D(B,C):
>         def spam(self, d): print(f'D: {d}')
> 
> The way to call all parent's functions exactly once (as per MRO) and accept
> all arguments and also forbid unknown arguments is to accept **kwargs
> everywhere and pass them to super().spam():
> 
>     class A:
>         def spam(self, a):
>             print(f'A: {a}')
>     class B(A):
>         def spam(self, b, **kwargs):
>             print(f'B: {b}')
>             super().spam(**kwargs)
>     class C(A):
>         def spam(self, c, **kwargs):
>             print(f'C: {c}')
>             super().spam(**kwargs)
>     class D(B, C):
>         def spam(self, d, **kwargs):
>             print(f'D: {d}')
>             super().spam(**kwargs)
> 
> Let's run this:
> 
>     >>> B().spam(a=1, b=2)
>     B: 2
>     A: 1
>     >>> D().spam(a=1, b=2, c=3, d=4)
>     D: 4
>     B: 2
>     C: 3
>     A: 1
> 
> You may notice that super() in B.spam refers to two different classes, either
> A or C, depending on inheritance order in yet undefinded classes (as of B's
> definition).
> 
> That's why the conclusion that super() in Callback.__init__ refers to object
> is wrong. In this example, spam=__init__, A=object, B=Callback and C and D are
> not yet written, but theoretically possible classes that could be written by
> someone else. Why would they be needed, I don't know, but if someone written
> them, s/he would be out of options to invent new arguments to C.__init__.
> 
> Note that super().__init__(*args, **kwargs) when super() refers to object
> isn't harmful, and just ensures that args and kwargs are empty (i.e. no
> unknown arguments were passed). In fact, this is exactly why object.__init__()
> takes no arguments since Python 2.6 [1][2], as you correctly point out in the
> commit message.
> 
> I don't think this breaks anything (I very much doubt anyone would need to
> write code that would trigger this), nevertheless, as the commit is both
> pointless and wrong, and as the original author of libvirtaio I'd like to ask
> for this commit to be reverted. If this breaks some static analysis tool,
> could you just suppress it for this particular line?

Could you open a merge request providing the revert along with your
description of why the change was wrong and I'll review & approve it.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|