Add support for serializing linear combinations of symbols #2358
Conversation
|
@dabacon PTAL. This has massively changed. |
…arsing # Conflicts: # cirq/google/op_deserializer.py
There was a problem hiding this comment.
Looks really good, few minor things to consider. I'd still leave it to @dstrain115 for the final approval :)
…arsing # Conflicts: # cirq/google/op_deserializer.py # cirq/google/op_serializer.py # cirq/google/serializable_gate_set.py
|
|
||
| # Argument types for gates. ArgFunction's are not currently supported. | ||
| ArgValue = Union[int, float, List[bool], str, sympy.Symbol] | ||
| SUPPORTED_FUNCTIONS_FOR_LANGUAGE: Dict[Optional[str], FrozenSet[str]] = { |
There was a problem hiding this comment.
nit: SUPPORTED_FUNCTIONS_PER_LANGUAGE might be clearer?
|
|
||
| msg = v2.program_pb2.Arg() if out is None else out | ||
|
|
||
| def check_support(func_type: str) -> str: |
There was a problem hiding this comment.
Is there a good reason to inline this? We have many other _internal functions in this file. I personally don't like inlined functions, but maybe it is just a style preference.
| @@ -12,9 +12,211 @@ | |||
| # See the License for the specific language governing permissions and | |||
| # limitations under the License. | |||
|
|
|||
| from typing import List, Union | |||
| from typing import ( | |||
There was a problem hiding this comment.
why the extra ','? If you remove that it will wrap nicer, right? For imports these long lists are annoying.
There was a problem hiding this comment.
Acknowledged. I prefer the vertical list style to the block style, as it makes git diffs cleaner.
|
|
||
| Args: | ||
| arg_proto: The proto containing a serialized value. | ||
| required_arg_name: If set to `None`, the method will return `None` when |
There was a problem hiding this comment.
nit: order does not match arg order
| Dict, | ||
| Optional, | ||
| Sequence, | ||
| TYPE_CHECKING, |
There was a problem hiding this comment.
nit: remove comma to get formatting that doesn't fill up my screen when I open the file :)
There was a problem hiding this comment.
Acknowledged. I prefer the vertical list style to the block style, as it makes git diffs cleaner.
There was a problem hiding this comment.
I think the white space is a big waste. Plus no one else is doing this in the code base.
Strilanc
added
the
automerge
CirqBot
added
the
front_of_queue_automerge
CirqBot
removed
the
automerge
CirqBot
removed
the
front_of_queue_automerge
No description provided.