Open tanujkhattar opened 4 months ago
For any subclasses of GateWithRegisters, you will need to add a #type: ignore[misc]
. This is because Bloq and cirq.Gate have conflicting definitions for the controlled method, so the double inheritance is not 100% valid accoridng to type checking.
For the third one, you probably need to change the return type to Iterator or Generator.
Let me know if you need help with this.
I thought cirq.OP_TREE
is defined to be a union of Operation
and an iterator over OpTree
so we shouldn't nee to change the return type to an iterator?
Either way, it looks like https://github.com/python/mypy/issues/731 is now fixed so we should update the definition of cirq.OP_TREE
to be a recursive type (cc @pavoljuhas can we do this before the next release?)
Either way, it looks like python/mypy#731 is now fixed so we should update the definition of
cirq.OP_TREE
to be a recursive type (cc @pavoljuhas can we do this before the next release?)
The problem with OP_TREE, whether defined as the current Union[Operation, OpTree]
or a recursive Union[Operation, Iterable[OP_TREE]]
type, is that it does not guarantee such types are iterable. You would need to add isinstance(optree, Iterable)
to pass mypy for every iteration over values that are returned as OP_TREE-s.
A proper way would be to redefine OP_TREE as Union[Iterable[Operation], Iterable[OP_TREE]]
, but I suspect that would necessitate a lot of changes to the cirq code and would be a breaking API change.
If I check cirq with mypy-1.10.0 as in qualtran I get a similar "Missing return statement" error for functions that yield after declaring an OP_TREE return type.
I suggest to change the return type of SelectSwapQROM.decompose_from_registers to Iterator[cirq.OP_TREE]
, there are already similar constructs in qualtran for example in TwoBitCSwap.decompose_from_registers.
https://github.com/quantumlib/Qualtran/pull/986 adds 3 type ignores to qrom bloqs to make mypy happy; but its not clear why these issues arise in the first place.
This issue is to unblock the PR and track the potential fixes of these mypy issues at a later point.
[ ] For the first two, there is some nuance going on with multiple inheritance and type-ignores in
controlled
method ofGateWithRegisters
. BothQROM
andSelectSwapQROM
derive fromGateWithRegisters
andQROMBase
(which is a new abstract base class). The only place where thecontrolled
method should come from is theGateWithRegisters
base class; but due to multiple inheritance mypy starts complaining. This issue doesn't arise for all other bloqs that only derive fromGateWithRegisters
. For some reason, having multiple parent classes is making mypy unhappy.[ ] For the third issue, we have used the pattern of yielding operations without an explicit "return" statement when a function returns a
cirq.OP_TREE
forever in cirq; but for some reason its making mypy unhappy here.cc @dstrain115