-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Implement ControlledGate #430
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
- Add extensions parameter to PotentialImplementation to allow recursive casting - Fix ParameterizableGate not being exported
The build failure on travis is some kind of mypy bug. Opened #431 to investigate. |
gate_features.ParameterizableGate, | ||
gate_features.ReversibleGate, | ||
gate_features.TextDiagrammableGate, | ||
) |
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.
Why are these particular subclasses in this list?
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.
Hmm I think I see ... these are the ones that you implemented methods for.
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.
Correct
if desired_type in POTENTIALLY_EXPOSED_SUB_TYPES: | ||
cast_sub_gate = ext.try_cast(self.sub_gate, desired_type) | ||
if cast_sub_gate is None: | ||
return None |
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.
Why not raise a TypeError, as you do in _cast_sub_gate
?
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.
Because this is a try_cast method, not a cast method.
This method is inherited from PotentialImplementation and so must satisfy that contract as opposed to one that may be more convenient to this specific class.
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.
Okay, I guess this is just following the docstring of try_cast_to
.
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.
Okay, I see.
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.
This is great! Naturally, supporting potential implementation of CompositeGate is tricky but this gives a path forward for someone who wants to do that.
Fixes #389
Fixes #431