Gate.on().gate should be consistent

[kevinsung]

Right now the return type of Gate.on is Operation, not GateOperation. I have come across situations where I want to assume that the result of Gate.on has a non-None gate attribute. This should be a safe assumption, but the types currently don't work out.

Changing the return type of Gate.on from Operation to GateOperation results in the following mypy errors:

cirq/ops/fourier_transform.py:194: error: Incompatible types in assignment (expression has type "Operation", variable has type "GateOperation")
cirq/ops/dense_pauli_string.py:268: error: Return type of "on" incompatible with supertype "Gate"
cirq/ops/controlled_gate.py:116: error: Return type of "on" incompatible with supertype "Gate"
cirq/ops/common_gates.py:845: error: Signature of "on" incompatible with supertype "Gate"
cirq/google/optimizers/convert_to_sycamore_gates.py:262: error: Argument 1 to "extend" of "list" has incompatible type "Iterator[Operation]"; expected "Iterable[GateOperation]"

I think the only tricky ones here are DensePauliString and ControlledGate. DensePauliString can be fixed by making PauliString a GateOperation. ControlledGate can be fixed by defining ControlledGateOperation. It makes sense to have both ControlledOperation and ControlledGateOperation given that both Operation and GateOperation exist.

[dabacon]

I've tried this refactor and indeed run into problems with ControlledOperation and PauliString. These feel like signs that these operations are not properly designed.

[dabacon]

I'm not sure if this is possible any more, but pre 1.0 should dedide if we want to do this.

[daxfohl]

I'd looked at this and it would be a pretty big change. Alternately, we may want a consistency test that gate == gate.on(...).gate that is automatically run on all gates defined in the project. This should work on all our gates now. The first sentence of this issue reflects that that is the actual problem, so perhaps update the issue title.

[dabacon]

OK so if we reduce the scope to testing this, I think the challenge is going to be generating instances of all of the gates. I thought about using the json test data for this, anyone have a better idea?

[dstrain115]

Adding discuss label to see if we want to do this. Given the comments and the corresponding difficulty of deprecating TaggedOperations, I think this is too big of an issue (with not as much benefit) to tackle before 1.0.

As suggested above, we could limit the scope to adding consistency that gate == gate.on(...).gate for all gates and then reduce the scope of the issue.

[dabacon]

#5354 adds tests for round tripping. There is at least one more gate that needs to be serialized to get complete coverage.

[dabacon]

Closing this issue as gates without json serialization is tracked in #5353