-
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
Stop raising AttributeError when accessing missing calibration #9681
Conversation
Thank you for opening a new pull request. Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient. While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone. One or more of the the following people are requested to review this:
|
Pull Request Test Coverage Report for Build 4296051502
💛 - 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.
This LGTM, it seems straightforward enough and is an obvious oversight we both made in #9597 (although I guess I naively assumed nobody would query the calibrations for something that doesn't have one :) ).
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> (cherry picked from commit 7463342)
#9683) Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> (cherry picked from commit 7463342) Co-authored-by: Naoki Kanazawa <[email protected]> Co-authored-by: Ikko Hamamura <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
…t#9681) Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Summary
In #9597 we updated the
InstructionProperties.calibration
to internally storeCalibrationEntry
subclass. However, there still be the caseNone
is stored instead when user/system doesn't provide any calibration, e.g.reset
instruction or any control flow operation. Accessing calibration of such entry raises AttributeError, which must be suppressed and it must returnNone
instead.Details and comments