-
Notifications
You must be signed in to change notification settings - Fork 569
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 tape.is_sampled and tape.all_sampled #4773
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4773 +/- ##
==========================================
- Coverage 99.64% 99.64% -0.01%
==========================================
Files 381 381
Lines 34308 34042 -266
==========================================
- Hits 34187 33920 -267
- Misses 121 122 +1
☔ View full report in Codecov by Sentry. |
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.
👍 Just a comment about the moved block on the deprecations page, but looks good to me :)
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.
🚀
Context:
tape.is_sampled
andtape.all_sampled
are not very useful and could really be evaluated on a per-call basis. Plus, maintaining the correctness of these values can be difficult if new measurements are introduced.Description of the Change:
Raise a warning whenever those properties are accessed.
Benefits:
We're getting rid of stuff that may not necessarily belong on the tape API! 🧹
Possible Drawbacks:
In the deprecation notice, we're suggesting the 4 measurement types that are currently being used, but they are theoretically subject to change. We discussed other things to recommend, but I think this is the best option so far because it's the same as what we use in
QubitDevice.execute
. Sure, it might change one day. But by then, the properties (and their warnings) will be removed altogether![sc-45870]