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

Fix type error in (X/Y)PowGate._act_on_ #3533

Merged
merged 5 commits into from Nov 25, 2020
Merged

Conversation

maffoo
Copy link
Contributor

@maffoo maffoo commented Nov 25, 2020

There was a type error due to the fact that the common superclass EigenGate does not define _act_on_. We can act on each gate instead of looping over them, which avoids the type error.

@maffoo maffoo requested review from smitsanghavi and a team November 25, 2020 18:06
@google-cla google-cla bot added the cla: yes Makes googlebot stop complaining. label Nov 25, 2020
Copy link
Collaborator

@smitsanghavi smitsanghavi left a comment

Choose a reason for hiding this comment

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

If we are changing this to avoid type: ignores, might as well fix the same pattern in YPowGate for consistency

@maffoo maffoo changed the title Fix type error in XPowGate._act_on_ Fix type error in (X/Y)PowGate._act_on_ Nov 25, 2020
Copy link
Contributor

@balopat balopat left a comment

Choose a reason for hiding this comment

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

This looks good - though I do like the list comprehension as it's a bit more python-esque.
An alternative way to keep it is to actually use the protocol type in the type definition:

                ops = [ZPowGate(), H]  # type: List[SupportsActOn]
                assert all(gate._act_on_(args) for gate in ops)

What do you think of this direction?

@maffoo
Copy link
Contributor Author

maffoo commented Nov 25, 2020

@balopat, I added a helper function to call _act_on_ on a series of gates. The helper has a type annotation as you suggested, which takes care of the types.

@maffoo maffoo added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Nov 25, 2020
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Nov 25, 2020
@CirqBot CirqBot merged commit 92e928e into master Nov 25, 2020
@CirqBot CirqBot removed the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Nov 25, 2020
@CirqBot CirqBot deleted the u/maffoo/xpow-act-on branch November 25, 2020 21:16
@CirqBot CirqBot removed the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Nov 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Makes googlebot stop complaining.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants