-
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
GroupParameter and Group (for visa commands that set/get more than one parameter at once) #1232
Conversation
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.
Sweet! thank you!
Codecov Report
@@ Coverage Diff @@
## master #1232 +/- ##
==========================================
+ Coverage 80.77% 80.85% +0.08%
==========================================
Files 49 50 +1
Lines 6841 6886 +45
==========================================
+ Hits 5526 5568 +42
- Misses 1315 1318 +3 |
…event "shaddowing" of the attributes in the base class.
…odes into feature/GroupParameter
… same signature as the base class
qcodes/instrument/group_parameter.py
Outdated
def __init__(self, name: str, instrument: Instrument, **kwargs) -> None: | ||
self.group: Union[Group, None] = None | ||
super().__init__(name, instrument=instrument, **kwargs) | ||
|
||
def set_raw(self, value: Any): | ||
self.group.set(self, value) | ||
self.set_raw = lambda value: self.group.set(self, value) |
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.
as we've discussed, i'm ok with reducing the code quality for the sake of keeping this code more readable. It's just the design of Parameter and _BaseParameter does not allow a clean way of redefining set_raw/get_raw. Once the design is fixed, the original implementation of GroupParameter will be correct (and most readable). Hence, I recommend to revert this commit. @jenshnielsen @WilliamHPNielsen
Merge: 1470658 b43ae7a Author: sohail chatoor <[email protected]> Merge pull request #1232 from QCoDeS/feature/GroupParameter
I have created a separate PR for the GroupParameter and added a basic test