PolynomialTensor._multiply has incorrect type signature

[kevinsung]

Environment

• Qiskit Nature version: N/A
• Python version: N/A
• Operating system: N/A

What is happening?

In https://github.com/Qiskit/qiskit-nature/blob/ba3a191b30606d6c1da5c378de2eea7bceaeab57/qiskit_nature/second_q/operators/polynomial_tensor.py#L331

other is typed as complex even though the code assumes it's a Number. I tried replacing Number with complex or Complex but that still gave errors; I think this is a bit delicate to fix.

How can we reproduce the issue?

N/A

What should happen?

Types should be correct.

Any suggestions?

No response

[woodsp-ibm]

Do you have some specific issue with the type? It follows the mixin definition from Terra (though there the type is specified on the arg)

https://github.com/Qiskit/qiskit-terra/blob/fca8db6b86cba106ed0449a99cd10223899b6145/qiskit/quantum_info/operators/mixins/multiply.py#L47-L52

and the multiply checks Number the same way

https://github.com/Qiskit/qiskit-terra/blob/fca8db6b86cba106ed0449a99cd10223899b6145/qiskit/quantum_info/operators/operator.py#L409-L422

[kevinsung]

Please see #891 (comment) for context. Let me know if the issue is still not clear.

[mrossinek]

Alright, let me try to summarize the situation of numeric type hints in Python. This is quite the convoluted rabbit hole, but I will try to keep it short and simple.

Overview

• A static type checker (like mypy) is unable to correctly understand the abstract classes from the numbers module. There is various convoluted reasons for this which you can read up on here but I will not go into the details again. Effectively this means, that using anything from numbers as a type hint will give you a bad type when using a static type checker.

• On the flip side, if you want to support not only builtin types like int, float, and complex, but also numpy or other derived types, you need to be very careful about possible isinstance-checks or other similar operations. With these, you will generally have the most flexible results when using numbers objects because these work as intended at runtime.

What does this mean for us?

• Generally I will try to avoid using numbers as part of type hints because this leads to a plethora of exceptions with mypy. This is also aligned with what is done in Terra most of the time (especially in the specific example of the LinearMixin which this issue was originally referring, too)

• However, at runtime I will try to use numbers wherever possible (and where I want to allow more flexibility than the type hint might suggest) in order to avoid issues related to derived types

While this causes a technical mismatch, this is a technicality. When I see a type supporting complex, I would expect any number that is also int, float, np.complex128, np.float64, or whatever to work fine, too. Yes, Number will allow even more at runtime, but where this is used as a check, the code below should also be fine with that.

So what do we do now?

I consider this issue a minor technicality which we should not get hung up on. Instead, I suggest that we continue with my outline in the previous section, and if we do ever encounter a bug specifically because of this, we can revisit this given the specific scenario (but I do not think that further theoretical discussion on this topic is needed).
One could potentially reflect this in the docstrings (i.e. that the actual type is a bit more loose); but as I also said above, this can also be seen as a common sense interpretation of the type hint complex (and besides that the docstrings could be improved more in general anyways).

Thus, I am closing this issue as per the above.