-
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
Serializers deprecation #5589
Serializers deprecation #5589
Conversation
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, minus two small comments.
@@ -175,7 +175,7 @@ def create_device_proto_for_qubits( | |||
gate = gs_proto.valid_gates.add() | |||
gate.id = gate_id | |||
|
|||
if not isinstance(serializer, op_serializer.GateOpSerializer): | |||
if not isinstance(serializer, op_serializer._GateOpSerializer): |
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.
This needs to handle both GateOpSerializer and _GateOpSerilalzer, right?
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.
_GateOpSerializer
is a superclass of GateOpSerializer
so both are handled
Thanks for the review, merging now. |
This PR deprecates all serializers we plan to deprecate * Deprecated GateOpSerializer, GateOpDeserializer, DeserializingArg, and SerializingArg using the class wrapper technique. * CircuitSerializer and serializer classes it depends on (`CircuitOp[De]Serializer`, `Serializer`) are left in place. * Skipping deprecation of common serializers in `common_serializers.py` for now and remove them directly after the 0.15 release as it's very unlikely for users to access them directly. Most users interact with them via global gatesets, e.g. https://github.com/quantumlib/Cirq/blob/abfc619bfe31e93573993c76cecfddc5689dea9b/cirq-google/cirq_google/serialization/gate_sets.py#L43-L64 But if time allows, I'll try to deprecate common serializers as well. * Added mentions of serializer deprecations in global gateset deprecation warnings. @dstrain115
This PR deprecates all serializers we plan to deprecate
CircuitOp[De]Serializer
,Serializer
) are left in place.common_serializers.py
for now and remove them directly after the 0.15 release as it's very unlikely for users to access them directly. Most users interact with them via global gatesets, e.g.Cirq/cirq-google/cirq_google/serialization/gate_sets.py
Lines 43 to 64 in abfc619
But if time allows, I'll try to deprecate common serializers as well.
@dstrain115