-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add bytes to cirq_google proto #7201
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
Conversation
dstrain115
commented
Mar 31, 2025
- Adds bytes to the cirq_google Arg proto
- Adds hooks in arg_func_langs to serialize and deserialize it.
- Adds bytes to the cirq_google Arg proto - Adds hooks in arg_func_langs to serialize and deserialize it.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7201 +/- ##
==========================================
- Coverage 98.14% 98.14% -0.01%
==========================================
Files 1100 1100
Lines 96147 96244 +97
==========================================
+ Hits 94365 94458 +93
- Misses 1782 1786 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Please see comments before merging.
@@ -291,6 +293,8 @@ def arg_from_proto( | |||
return [str(v) for v in arg_value.string_values.values] | |||
if which_val == 'value_with_unit': | |||
return tunits.Value.from_proto(arg_value.value_with_unit) | |||
if which_val == 'bytes_value': | |||
return bytes(arg_value.bytes_value) |
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.
Note (no change necessary) - perhaps these iffs could be converted to a match-case block.
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.
SGTM, maybe in a follow-up PR.
@@ -55,6 +56,14 @@ def _json_format_kwargs() -> Dict[str, bool]: | |||
[ | |||
(1.0, {'arg_value': {'float_value': 1.0}}), | |||
(1, {'arg_value': {'float_value': 1.0}}), | |||
( | |||
bytes('abcdef', 'utf-8'), |
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.
Nit - consider replacing bytes('abcdef', 'utf-8')
--> b'abcdef'
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.
Done.
* Add bytes to cirq_google proto - Adds bytes to the cirq_google Arg proto - Adds hooks in arg_func_langs to serialize and deserialize it. * Changed bytes to b