-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Migrate contrib json tests #7742
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
Migrate contrib json tests #7742
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
pavoljuhas
left a comment
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.
Mostly there after few adjustments. Please see inline comments.
cirq-core/cirq/contrib/json.py
Outdated
| self._warning_shown = True | ||
|
|
||
|
|
||
| DEFAULT_CONTRIB_RESOLVERS = _DeprecatedDefaultContribResolvers() |
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 use _compat.deprecate_attributes with a deadline of v1.8 instead - similarly as what was done for the XMON (and others) here -
Cirq/cirq-google/cirq_google/__init__.py
Lines 175 to 184 in 7e67d66
| _compat.deprecate_attributes( | |
| __name__, | |
| { | |
| 'XMON': ('v0.16', _SERIALIZABLE_GATESET_DEPRECATION_MESSAGE), | |
| 'FSIM_GATESET': ('v0.16', _SERIALIZABLE_GATESET_DEPRECATION_MESSAGE), | |
| 'SQRT_ISWAP_GATESET': ('v0.16', _SERIALIZABLE_GATESET_DEPRECATION_MESSAGE), | |
| 'SYC_GATESET': ('v0.16', _SERIALIZABLE_GATESET_DEPRECATION_MESSAGE), | |
| 'NAMED_GATESETS': ('v0.16', _SERIALIZABLE_GATESET_DEPRECATION_MESSAGE), | |
| }, | |
| ) |
The deprecate_attributes call should be the last one in this module.
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.
Hi @pavoljuhas
I have tried creating similar to the XMON example provided, can you please review the change and let me know if it is correct or is there any fix needed for the change?
I tried running a basic run with test code but since json_test.py has been removed, I wasn't able to confirm the warning message.
| # TODO: #7520 - create .json and .repr for these so they can be tested here | ||
| tested_elsewhere=["QuantumVolumeResult", "SwapPermutationGate", "BayesianNetworkGate"], | ||
| #tested_elsewhere=["QuantumVolumeResult", "SwapPermutationGate", "BayesianNetworkGate"], | ||
| tested_elsewhere=[] |
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 delete, tested_elsewhere is an optional argument.
Also clean up the TODO comment, it would be confusing if left in.
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.
Hi @pavoljuhas
I have removed tested_elsewhere from the spec.py file
|
@RahilJain1366 - please update your PR to merge towards the main branch. The vX.Y.Z-dev branches are for releases only and should not be use for regular code development. (If easier you can also close this PR, rebase your branch on |
|
I have created a new PR (#7743) merging to the main branch. |
No description provided.