Deprecate convenience methods on Instrument classes
Following the discussion in #6080 I think we should consider getting rid of these methods on the instrument classes.
All of this functionality can be done in ways that is more obvious, direct and more frequently used
E.g. parameter and functions can be looked up from the parameters and functions delegate attr dicts if you need to look them up dynamically.
- [x] Fix any use of this in qcodes
- [ ] Should check agains contrib drivers and perhaps zhinst-qcodes and others
- [ ] Newsfragment if we want to merge this
Codecov Report
Attention: Patch coverage is 77.77778% with 2 lines in your changes missing coverage. Please review.
Project coverage is 67.34%. Comparing base (
badc122) to head (f02bdc2). Report is 14 commits behind head on main.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| src/qcodes/instrument_drivers/tektronix/AWG5014.py | 50.00% | 2 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #6086 +/- ##
==========================================
- Coverage 67.35% 67.34% -0.01%
==========================================
Files 352 352
Lines 32139 32143 +4
==========================================
+ Hits 21646 21647 +1
- Misses 10493 10496 +3
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
I support this deprecation. The only usefulness related to this that I can recall is calling parameters/methods/functions on channel lists, but that I already implemented and work for both proper parameters and bound methods (right?)
Yes, there was a limitation where you could not call a method on more than one channel (which was an advantage of a qcodes function) but that has since been lifted.
This deprecation results in quite some warnings in our code. I agree that the code is cleaner, but I don't like it that code used in the labs will break in many places when this functionality is removed.
@sldesnoo-Delft Thanks for your input. In the code that I had access to I saw very few uses of these patterns so good to know they are being used elsewhere. I would suggest that we leave this code deprecated but there is no reason that we have to remove it any time soon if there is real usage of it. That way users will be guided in the right direction.
BTW since we are using the new 3.13 deprecated decorator (backported from typing extensions) you can use a type checker for catching most usage of deprecated code. I have been using pyright with a config like this (pyright.json)
{
"typeCheckingMode": "off",
"reportDeprecated": true
}
as a quick way to find all calls to deprecated methods etc.
I believe that mypy is also in the process of adding support for this in the next release or so.
I just want to chime in here as someone from a lab that has built a software package where many objects (qubits, AWGs, etc.) are instances of qcodes InstrumentBase. This change results in a lot of deprecation warnings at once. Many measurements may now emit more than 10 of these.
While we will refactor the several hundred instances of get/set, I would like to understand the reasoning here a bit better. Are there any performance or other technical reasons for this change, or is it rather one of code organization? #6080 doesn't seem to explicitly discuss get and set, but rather focuses on indexing.
I also briefly note that these methods are not shown as deprecated in the API documentation
@grahamnorris
Thanks for your message #6657 will add the deprecation to the docstrings.
We made this deprecation since these methods do not add any value to qcodes. The main aim is to make it easier for users to
Some of the issues
- Looking up a parameter by name is not type safe unlike using the attribute. As you cannot know the type of instrument["myparam"]
- They are surprising and not very pytonic. E.g. In python you don't look up attributes by name on classes using
__getitem__/[]That is something that is reserved for dicts/mappings. You dont call an object by passing the name of that object to a function etc. - They violate the zen of python.
There should be one-- and preferably only one --obvious way to do it.Since they add an additional way to do things and they are not obvious.
Note that as with all other python warnings you have the option to silence them until you have the time to perform a transition.
https://docs.python.org/3/library/warnings.html#the-warnings-filter