[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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?


[0] https://gitlab.com/libvirt/libvirt-python/-/commit/f4be03b330125ab1e5a2bb10b4f12674aeff4691
[1] https://bugs.python.org/issue1683368
[2] https://docs.python.org/3/whatsnew/2.6.html#porting-to-python-2-6
    (fourth point)

-- 
pozdrawiam / best regards
Wojtek Porczyk
Invisible Things Lab
 
 I do not fear computers,
 I fear lack of them.
    -- Isaac Asimov

Attachment: signature.asc
Description: PGP signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]