Qiskit-terra: Using a numpy integer type as an index for a QuantumRegister fails

[bjader]

Information

• Qiskit Terra version: 0.11.1
• Python version: 3.7.6
• Operating system: Ubuntu 18.04.4 LTS

What is the current behavior?

An error is raised:

File "/lib/python3.7/site-packages/qiskit/circuit/register.py", line 90, in __getitem__
    raise CircuitError("expected integer or slice index into register")
qiskit.circuit.exceptions.CircuitError: 'expected integer or slice index into register'

Steps to reproduce the problem

from qiskit import QuantumRegister
import numpy as np
qr = QuantumRegister(3)
qubit_index = np.int64(0)
qubit = qr[qubit_index]

What is the expected behavior?

Since numpy is used extensively in scientific programming, the type checking should not be as strict.

Suggested solutions

Change line 89 of register.py from:

if not isinstance(key, (int, slice, list)):

to

if not isinstance(key, (int, slice, list, np.integer)):

[mtreinish]

@bjader I agree, please feel free to push a PR with your suggested change (probably with a unit test covering passing a numpy int in), it'll be an easy approval :)

[bjader]

Great, I'll get round to it over the next few days! Looking forward to my first PR

[nonhermitian]

Why not use the Python numbers library here?

[bjader]

@nonhermitian In what way? Does the numbers library have a common type between python ints and numpy ints? I'm not familiar with the library.

Edit: Aha, as such?

isinstance(np.int64(0), numbers.Number)
Out[31]: True

[nonhermitian]

Everything should be a numbers.Number

[1ucian0]

But floats and complex are also numbers.

I think the approach should be trying casting to int before the type check. My 2 cents.

[kdk]

isinstance(foo, numbers.Integral) and 0 <= foo < len(self) should cover those cases.

Agree, in general, try/catch is more pythonic (and would simplify implementation), but would leave gaps for users who are new to python. e.g. int(3.5) == 3.

[Cryoris]

Actually, numbers.Integral evaluates to False if passed a complex. So using Integral should be fine I think.

[bjader]

If another user wants to make the change / pull request please feel free. Unfortunately, I've become swamped since the pandemic.

[arunraja-hub]

Hi, I am keen to contribute to solving this as my first issue. Can I work on this?

[Cryoris]

Yes that would be great! Do you know where to start or do you need a hand? 🙂

[arunraja-hub]

Thank you!
I changed line 120/121 of register.py from:

if not isinstance(key, (int, slice, list)):

to

if not isinstance(key, (int, slice, list, np.integer)):

as suggested above and it works:

Can I make a pull request?

[Cryoris]

Can you use numbers.Integral instead, as it also covers other integer types? I.e.

if not isinstance(key, (numbers.Integral, slice, list)):

[arunraja-hub]

Yes numbers.Integral works too

[Cryoris]

Then you can open a PR 👍

[arunraja-hub]

Thank you, just opened a PR!