-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Return fast from transpile with empty input #7288
Conversation
Pull Request Test Coverage Report for Build 1478066876
💛 - Coveralls |
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.
Overall LGTM, just one quick question inline about potential side effects.
@@ -240,6 +240,9 @@ def callback_func(**kwargs): | |||
arg_circuits_list = isinstance(circuits, list) | |||
circuits = circuits if arg_circuits_list else [circuits] | |||
|
|||
if not circuits: |
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.
Are you concerned at all about the side effects of transpile(0),
transpile(False)or
transpile(None)` here? I only ask because if a user did any of those I'd expect an error.
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.
Right before these lines, circuits
is promoted to a 1-element list if it isn't one already, so in those cases, it'd all be promoted to circuits = [0]
, circuits = [False]
, etc. I didn't check to see if super invalid output fails somewhere else, but the boolean coercion shouldn't cause any more problems here.
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.
Ok cool I see that now on L241. Heh I should have looked at more than just strict diff to see the surrounding context.
Summary
Previously
transpile([])
raised a warning. Now we just return quickly without further tests, similarly to how we return early if we're asked to transpile a list ofSchedule
s.Details and comments
Fix #7287.