-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Flush v0.14.0 deprecation backlog. #4601
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confused about the repr_inwards
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't remove the json_inward files
If I get rid of the repr_inward and leave just the json_inward, the json_inward tests complain there is no corresponding repr_inward file. I think I either need to keep both and do the rename in the |
If the json_inward tests require the repr_inward files, then yes: keep them and use the new cirq_google spelling therein |
0b150f9
to
2dc7527
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no blockers from me
@@ -112,7 +116,7 @@ def two_qubit_matrix_gate(matrix): | |||
'_PauliY': cirq.ops.pauli_gates._PauliY, | |||
'_PauliZ': cirq.ops.pauli_gates._PauliZ, | |||
'ParamResolver': cirq.ParamResolver, | |||
'ParallelGateOperation': cirq.ParallelGateOperation, | |||
'ParallelGateOperation': _parallel_gate_op, # Removed in v0.14 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI: The two classes are not strictly interchangeable (i.e. ParallelGateOperation
vs ParallelGate().on()
) and can result in breakages in the user code after this substitution. Though I'm not sure we have a better backward compatible alternative at this point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add a breaking change tag and outline some more details here so we remember for next release ?
"outputs": [ | ||
{ | ||
"name": "stdout", | ||
"output_type": "stream", | ||
"text": [ | ||
"Circuit with custom gates:\n", | ||
"0: ───G───\n" | ||
] | ||
} | ||
], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it expected to get rid of these output cells? It would change the way these colabs are rendered on the website?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It won't change the way the notebooks are rendered on the site. If the outputs on the notebook are given, then the site will use those, otherwise it will just run the notebook and use that output. Typically we'd only want fixed outputs if things needed it due to things like randomness in the notebook or certain outputs looking bad. I couldn't see a reason why we needed the fixed output here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM % nits.
I find the breaking change label funny here. Clearly removing a host of stuff is a breaking change, albeit through the deprecation cycle. I guess you want to call attention to the fact that one of the deprecated items doesn't have a lossless, automated migration? |
Removed `cirq.<vendor>` to `cirq_<vendor>` deprecations (This required some repr_inward changes as well as some renaming in `cirq_pasqal`). Removed `ParallelGateOperation`. Removed `cirq.Two/ThreeQubitGate`. We still have `cirq.testing.Two/ThreeQubitGate`. Removed `SupportsOnEachGate`. Removed `allow_decompose` parameter from `measurement_key_names` and `is_measurement`. Removed `kraus_to_channel_matrix`, `operation_to_channel_matrix`. Removed `asynchronous_pending` and `cirq-core/cirq/testing/asynchronous.py`. Removed `gate_set_name` in favor of `name` from `serializable_gate_set.py` @mpharrigan can you double check my changes on the json items. BREAKING CHANGE= some features of ParallelGateOperation aren't fully convertible. i.e. `cirq.num_qubits(old_op.gate) != cirq.num_qubits(new_op.gate)` and `isinstance(old_op, GateOperation) != isinstance(new_op, GateOperation)`
Removed `cirq.<vendor>` to `cirq_<vendor>` deprecations (This required some repr_inward changes as well as some renaming in `cirq_pasqal`). Removed `ParallelGateOperation`. Removed `cirq.Two/ThreeQubitGate`. We still have `cirq.testing.Two/ThreeQubitGate`. Removed `SupportsOnEachGate`. Removed `allow_decompose` parameter from `measurement_key_names` and `is_measurement`. Removed `kraus_to_channel_matrix`, `operation_to_channel_matrix`. Removed `asynchronous_pending` and `cirq-core/cirq/testing/asynchronous.py`. Removed `gate_set_name` in favor of `name` from `serializable_gate_set.py` @mpharrigan can you double check my changes on the json items. BREAKING CHANGE= some features of ParallelGateOperation aren't fully convertible. i.e. `cirq.num_qubits(old_op.gate) != cirq.num_qubits(new_op.gate)` and `isinstance(old_op, GateOperation) != isinstance(new_op, GateOperation)`
Removed
cirq.<vendor>
tocirq_<vendor>
deprecations (This required some repr_inward changes as well as some renaming incirq_pasqal
).Removed
ParallelGateOperation
.Removed
cirq.Two/ThreeQubitGate
. We still havecirq.testing.Two/ThreeQubitGate
.Removed
SupportsOnEachGate
.Removed
allow_decompose
parameter frommeasurement_key_names
andis_measurement
.Removed
kraus_to_channel_matrix
,operation_to_channel_matrix
.Removed
asynchronous_pending
andcirq-core/cirq/testing/asynchronous.py
.Removed
gate_set_name
in favor ofname
fromserializable_gate_set.py
@mpharrigan can you double check my changes on the json items.
BREAKING CHANGE= some features of ParallelGateOperation aren't fully convertible. i.e.
cirq.num_qubits(old_op.gate) != cirq.num_qubits(new_op.gate)
andisinstance(old_op, GateOperation) != isinstance(new_op, GateOperation)