-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Deprecate passing None
to DAGCircuit
appenders
#10752
Conversation
This has been against the documented typing of the functions for some time, but some scheduling methods have mistakenly been passing `None`. There is no need to support `None` here; the empty tuple `()` is equally immutable and a CPython singleton, so there are neither mutability nor memory benefits to supporting both, and removing `None` as an input is a simple way to remove a(n admittedly cheap) branch from the appender methods.
One or more of the the following people are requested to review this:
|
Pull Request Test Coverage Report for Build 6039053966
💛 - 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.
Very small comment (actually, not a breaking change, I'm taking it back) the rest is ready to be merged
9611b77
to
7502f16
Compare
That latest merge commit didn't quite work right - I've just forced pushed a fixed merge commit in 7502f16. |
Summary
This has been against the documented typing of the functions for some time, but some scheduling methods have mistakenly been passing
None
. There is no need to supportNone
here; the empty tuple()
is equally immutable and a CPython singleton, so there are neither mutability nor memory benefits to supporting both, and removingNone
as an input is a simple way to remove a(n admittedly cheap) branch from the appender methods.Details and comments
I also made Qiskit/qiskit-ibm-provider#721 to fix the same mistake that the IBM Provider inherited from Terra.