-
Notifications
You must be signed in to change notification settings - Fork 322
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
API: Add DelatedKeyboardInterrupt and depreacate to api #4309
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4309 +/- ##
==========================================
- Coverage 68.44% 68.43% -0.01%
==========================================
Files 254 254
Lines 30969 30967 -2
==========================================
- Hits 21196 21192 -4
- Misses 9773 9775 +2 |
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.
The deprecate module contains 2 methods to assert deprecation messages. One of those were used in test_station. I have replaced that with the warnings testing logic from pytest. I think that using the pytest code is cleaner and more portable so I suggest that we only use that going forward and don't document these methods.
agree with using pytest's syntax and don't expose those assertion methods as public api.
test_station still makes use of the function to generate a test message deprecation_message I don't however think this should be part of the public api. We could rewrite the tests to not make use of this function?
deprecation_message indeed should not be part of the qcodes api for use outside of qcodes. our users should only want to silence/catch/etc the specific qcodes warning, and maybe use the convenient issue warning and decorate functions (but that I could also doubt about). However, i don't see any harm in using this function INSIDE qcodes, even in the tests (where actually matching on the "message" and "alternative" parts of the warning would be enough).
Co-authored-by: Mikhail Astafev <[email protected]>
bors merge |
A few questions
deprecation_message
I don't however think this should be part of the public api. We could rewrite the tests to not make use of this function?