Skip to content
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

Disallow empty measurement keys and fix tests using empty keys #4060

Merged
merged 4 commits into from Aug 2, 2021

Conversation

smitsanghavi
Copy link
Collaborator

Part of #4040

@google-cla google-cla bot added the cla: yes Makes googlebot stop complaining. label Apr 28, 2021
@smitsanghavi
Copy link
Collaborator Author

@95-martin-orion

@95-martin-orion 95-martin-orion requested review from 95-martin-orion and removed request for viathor May 4, 2021 16:33
@95-martin-orion
Copy link
Collaborator

Discussed offline - this may not be necessary if we choose to auto-convert keyless gates into qubit-named gates. Holding off on merge until @smitsanghavi confirms his decision.

@balopat
Copy link
Contributor

balopat commented Jul 1, 2021

ping @95-martin-orion @smitsanghavi is this to be closed or pushed forward?

@95-martin-orion
Copy link
Collaborator

Closing this as we had opted for a different solution (auto-convert "keyless" gates to qubit-based naming). If we need to revisit this (or decide that this PR was the correct choice) we can reopen this PR.

@smitsanghavi smitsanghavi reopened this Jul 31, 2021
@smitsanghavi
Copy link
Collaborator Author

Reviving this since we decided to revert the qubit based auto-naming.

Copy link
Collaborator

@95-martin-orion 95-martin-orion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. IIUC, all replace calls go through the constructor, so any accidental clearing of key strings by remapping will be prevented.

@95-martin-orion 95-martin-orion merged commit 409a412 into quantumlib:master Aug 2, 2021
@dstrain115 dstrain115 added the BREAKING CHANGE For pull requests that are important to mention in release notes. label Aug 19, 2021
rht pushed a commit to rht/Cirq that referenced this pull request May 1, 2023
…umlib#4060)

* Empty keys are illegal now

* merge

* fix duplicate post-inits
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BREAKING CHANGE For pull requests that are important to mention in release notes. cla: yes Makes googlebot stop complaining.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants