search

pedrorrivero / qrand

A multiprotocol and multiplatform quantum random number generation framework
https://pypi.org/project/qrand/
Apache License 2.0
23 stars 14 forks source link

Use of a deprecated syntax for `super()` #29

Closed LaurentAjdnik closed 2 years ago

LaurentAjdnik commented 2 years ago

Describe the bug

The QiskitCircuit class contains 21 occurrences of super(QuantumCircuit, self).

With a restrictive linter, this generates 21 error-level messages Bad first argument 'QuantumCircuit' given to super().

PEP 3135 introduced a new syntax for super(), where the child class calling super() should not be mentioned any longer. See here and here.

Unless there is a reason for using this ancient syntax?

To Reproduce

Steps to reproduce the behavior:

  1. Open QiskitCircuit with a restrictive linter, such as pylint.

Expected behavior

No error messages thrown by the linter (that is no use of the ancient syntax).

Screenshots

Not appllicable.

Desktop (please complete the following information):

Additional context

Not applicable.

pedrorrivero commented 2 years ago

The main reason for using that syntax is to address multiple inheritance. Notice that the class that you mention has two parent classes, so we need to be able to specify which class we refer to when calling super(). I am not aware of any other way of dealing with this, but let me know if you have any suggestions.

Edit: This may be a good alternative.

LaurentAjdnik commented 2 years ago

Notice that the class that you mention has two parent classes, so we need to be able to specify which class we refer to when calling super().

Never used this syntax but... I'm not sure this is what's happening.

From Python 2.7 docs on super() and my other links above, I understand that the first parameter must always be set to the calling class, not to a parent class we intend to favor.

Priorities are given by the MRO technique, which is independent from this (it relies mostly on the order in which inherited classes are declared).

In fact, there seems to be absolutely no functional difference between the "old-with-parameters" (Python 2) and the "mandatory-new-and-without-parameters" (Python 3+) syntax. Which tends to prove that the first parameter has absolutely no impact on MRO resolution.

Anyhow, coming from languages that strictly forbid multiple inheritance, I tend to always avoid it. It's usually only an endless source of trouble. 🤯

How important is it to inherit from QiskitQuantumCircuit? Couldn't it be turned into an attribute? (= Good old "Composition instead of inheritance")

(Whereas inheriting all specific classes from the generic QuantumCircuit makes perfect sense).

pedrorrivero commented 2 years ago

It is absolutely vital to inherit from QiskitQuantumCircuit since we are expanding that class and need its base functionality. Composition is not what we are looking for (that was my first guess as well but it failed).

The issue here is that in other languages you can implement several interfaces and inherit from a class as well. Python does not have such a thing as an interface (i.e. for QuantumCircuit), so all we are left with is multiple inheritance from abstract classes. It is not nice, I agree, but that is how python seems to be designed anyway.

I was using the old syntax because that seemed to be the only way around the MRO (we need to skip one class in the hierarchy). Nonetheless, I just found that there seems to be a slightly better approach, so I will revisit this and try to update to the alternative syntax ParentClass.method(self, *args, **kwargs) from my previous response. Any thoughts?

As for this:

Never used this syntax but... I'm not sure this is what's happening.

From Python 2.7 docs on super() and my other links above, I understand that the first parameter must always be set to the calling class, not to a parent class we intend to favor.

Priorities are given by the MRO technique, which is independent from this (it relies mostly on the order in which inherited classes are declared).

In fact, there seems to be absolutely no functional difference between the "old-with-parameters" (Python 2) and the "mandatory-new-and-without-parameters" (Python 3+) syntax. Which tends to prove that the first parameter has absolutely no impact on MRO resolution.

If you try removing the arguments in super() you will see that the code breaks under execution, so it is definitely not equivalent in this case. What you pass on is not the class whose method you want to execute, but the class where you want super to start the MRO (not included). You can read about it here.

pedrorrivero commented 2 years ago

Update: the ParentClass.method(self, *args, **kwargs) is not valid for properties. I will try changing the order of inheritance.