Add __radd__ in Circuit

[jaeyoo]

This commit adds radd in cirq.Circuit() because Moment + Circuit doesn't work even if Circuit + Moment works.

[c-poole]

LGTM! Thanks for the contribution. We'll need an approval from someone with write access before it can be merged, but it shouldn't take too long.

[jaeyoo]

LGTM! Thanks for the contribution. We'll need an approval from someone with write access before it can be merged, but it shouldn't take too long.

I really appreciate for helping me ramping up for this first PR! :D

[vtomole]

Add a test that covers

raise ValueError("Can't add circuits with incompatible devices.")

to pass the coverage check.and i'll merge it it

[dabacon]

Please add tests for these lines;

Line  277 is new or changed but not covered:                  return NotImplemented
Line  286 is new or changed but not covered:              raise ValueError("Can't add circuits with incompatible devices.")

[dabacon]

Missing coverage for two lines (See my other comment)

[Strilanc]

Nit @dabacon : You don't need to request changes that the build system is already enforcing.

[c-poole]

Nit @dabacon : You don't need to request changes that the build system is already enforcing.

Perhaps not, but it is a helpful reminder for a first-time contributor.

[Strilanc]

Yeah, the explanation is certainly a good idea.

[jaeyoo]

Yes, I need to learn more about github system because I am a novice. Thank you all for your kind comments. I will work on the coverage!

[jaeyoo]

I have a question. I realized that radd doesn't require device checking codes inside because (1) add already has it and (2) moment + circuit is converted into cirq.Circuit(moment) + self, which is able to be concatenated with any device. (3) there is no way to set the device of moment, except (4) cirq.Circuit(moment, device=cg.Foxtail) explicitly, for example, then (5) it runs add, not radd.

In conclusion, may I just remove the device comparison codes inside radd?

[c-poole]

If the error line that the tests don't currently cover is unreachable then the checks that trigger that error are unnecessary and can be removed. You should be able to remove the device checking in __radd__ without introducing the risk of adding circuits with incompatible devices.

[Strilanc]

Use backticks ` to avoid the underscores becoming bold __like_this__ instead of like_this.

Yes, if the other instance is a Circuit then __add__ will be used instead. You can start __radd__ with a comment # The Circuit case is handled by __add__, and then just do an insert at index 0.

(This is an example of the code coverage check doing a good thing. It effectively noticed the case was impossible!)

[jaeyoo]

Thank you, but I am stuck with coverage. I am very confused. Even if I added test codes, coverage still says my codes are not covered. On top of it, it also says my test codes are not covered! Why are my test codes recognized as tests? LOL. I just ran "./check/pytest-and-incremental-coverage" and the messages are:

Running pytest [23/337]
ERROR: usage: pytest [options] [file_or_dir] [file_or_dir] [...]
pytest: error: unrecognized arguments: --benchmark-skip
inifile: None
rootdir: /usr/local/google/home/jaeyoo/git/Cirq

Finished pytest
Running incremental-coverage
************* cirq/circuits/circuit.py (4 uncovered)
Line 276 is new or changed but not covered: if not isinstance(other, (ops.Operation, Iterable)):
Line 277 is new or changed but not covered: return NotImplemented
Line 279 is new or changed but not covered: result = Circuit(other)
Line 280 is new or changed but not covered: return result.iadd(self)
************* cirq/circuits/circuit_test.py (25 uncovered)
Line 252 is new or changed but not covered: a = cirq.NamedQubit('a')
Line 253 is new or changed but not covered: b = cirq.NamedQubit('b')
Line 255 is new or changed but not covered: c = cirq.Circuit()
Line 256 is new or changed but not covered: assert [cirq.X(a), cirq.Y(b)] + c == cirq.Circuit([
Line 257 is new or changed but not covered: cirq.Moment([cirq.X(a), cirq.Y(b)]),
Line 258 is new or changed but not covered: ]) [4/337]
Line 260 is new or changed but not covered: assert cirq.X(a) + c == cirq.Circuit(cirq.X(a))
Line 261 is new or changed but not covered: assert [cirq.X(a)] + c == cirq.Circuit(cirq.X(a))
Line 262 is new or changed but not covered: assert [[[cirq.X(a)], []]] + c == cirq.Circuit(cirq.X(a))
Line 263 is new or changed but not covered: assert (cirq.X(a),) + c == cirq.Circuit(cirq.X(a))
Line 264 is new or changed but not covered: assert (cirq.X(a) for _ in range(1)) + c == cirq.Circuit(cirq.X(a))
Line 265 is new or changed but not covered: with pytest.raises(AttributeError):
Line 266 is new or changed but not covered: _ = cirq.X + c
Line 267 is new or changed but not covered: with pytest.raises(TypeError):
Line 268 is new or changed but not covered: _ = 0 + c
Line 271 is new or changed but not covered: d = cirq.Circuit()
Line 272 is new or changed but not covered: d.append(cirq.Y(b))
Line 273 is new or changed but not covered: assert [cirq.X(a)] + d == cirq.Circuit([
Line 274 is new or changed but not covered: cirq.Moment([cirq.X(a)]),
Line 275 is new or changed but not covered: cirq.Moment([cirq.Y(b)])
Line 276 is new or changed but not covered: ])
Line 277 is new or changed but not covered: assert cirq.Moment([cirq.X(a)]) + d == cirq.Circuit([
Line 278 is new or changed but not covered: cirq.Moment([cirq.X(a)]),
Line 279 is new or changed but not covered: cirq.Moment([cirq.Y(b)])
Line 280 is new or changed but not covered: ])

[jaeyoo]

Thank you, but I am stuck with coverage. I am very confused. Even if I added test codes, coverage still says my codes are not covered. On top of it, it also says my test codes are not covered! Why are my test codes recognized as tests? LOL. I just ran "./check/pytest-and-incremental-coverage" and the messages are:

Running pytest [23/337]
ERROR: usage: pytest [options] [file_or_dir] [file_or_dir] [...]
pytest: error: unrecognized arguments: --benchmark-skip
inifile: None
rootdir: /usr/local/google/home/jaeyoo/git/Cirq

Finished pytest
Running incremental-coverage
************* cirq/circuits/circuit.py (4 uncovered)
Line 276 is new or changed but not covered: if not isinstance(other, (ops.Operation, Iterable)):
Line 277 is new or changed but not covered: return NotImplemented
Line 279 is new or changed but not covered: result = Circuit(other)
Line 280 is new or changed but not covered: return result.iadd(self)
************* cirq/circuits/circuit_test.py (25 uncovered)
Line 252 is new or changed but not covered: a = cirq.NamedQubit('a')
Line 253 is new or changed but not covered: b = cirq.NamedQubit('b')
Line 255 is new or changed but not covered: c = cirq.Circuit()
Line 256 is new or changed but not covered: assert [cirq.X(a), cirq.Y(b)] + c == cirq.Circuit([
Line 257 is new or changed but not covered: cirq.Moment([cirq.X(a), cirq.Y(b)]),
Line 258 is new or changed but not covered: ]) [4/337]
Line 260 is new or changed but not covered: assert cirq.X(a) + c == cirq.Circuit(cirq.X(a))
Line 261 is new or changed but not covered: assert [cirq.X(a)] + c == cirq.Circuit(cirq.X(a))
Line 262 is new or changed but not covered: assert [[[cirq.X(a)], []]] + c == cirq.Circuit(cirq.X(a))
Line 263 is new or changed but not covered: assert (cirq.X(a),) + c == cirq.Circuit(cirq.X(a))
Line 264 is new or changed but not covered: assert (cirq.X(a) for _ in range(1)) + c == cirq.Circuit(cirq.X(a))
Line 265 is new or changed but not covered: with pytest.raises(AttributeError):
Line 266 is new or changed but not covered: _ = cirq.X + c
Line 267 is new or changed but not covered: with pytest.raises(TypeError):
Line 268 is new or changed but not covered: _ = 0 + c
Line 271 is new or changed but not covered: d = cirq.Circuit()
Line 272 is new or changed but not covered: d.append(cirq.Y(b))
Line 273 is new or changed but not covered: assert [cirq.X(a)] + d == cirq.Circuit([
Line 274 is new or changed but not covered: cirq.Moment([cirq.X(a)]),
Line 275 is new or changed but not covered: cirq.Moment([cirq.Y(b)])
Line 276 is new or changed but not covered: ])
Line 277 is new or changed but not covered: assert cirq.Moment([cirq.X(a)]) + d == cirq.Circuit([
Line 278 is new or changed but not covered: cirq.Moment([cirq.X(a)]),
Line 279 is new or changed but not covered: cirq.Moment([cirq.Y(b)])
Line 280 is new or changed but not covered: ])

Oh, I realized that I didn't install pytest-benchmark in my venv. As well, I missed registering upstream yet. Both make coverage happy & green now :D Thank you for your helps! Now I got it!

[jaeyoo]

Missing coverage for two lines (See my other comment)

I've done. Could you give me LGTM?

[vtomole]

Love the simpler implementation.

[CirqBot]

Automerge cancelled: Merging is blocked (I don't understand why).

Coverage was added