-
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
Allow Delegate parameter to change source and support a None source #2005
Allow Delegate parameter to change source and support a None source #2005
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2005 +/- ##
==========================================
+ Coverage 71.17% 71.21% +0.04%
==========================================
Files 149 149
Lines 19541 19607 +66
==========================================
+ Hits 13909 13964 +55
- Misses 5632 5643 +11 |
4bb6311
to
473dc85
Compare
8eefca2
to
2893bda
Compare
by implementing a cacheprotocol that both cache classes implement
ce592c8
to
88e3f37
Compare
qcodes/instrument/parameter.py
Outdated
if self._parameter.source is None: | ||
raise TypeError("Cannot set the cache of a DelegateParameter " | ||
"that delegates to None") | ||
self._parameter.validate(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 per my memory, _set_from_raw_value was made with an assumtion that it will be used in a "get" context (as opposed to "set"). this is why in _BaseParameter, _set_from_raw_value
validates the value only if _validate_on_get is True. I don't think that this assumtion is a good one. So either the _validate_on_get condition needs to be gone but then we need to be careful about validation values in group parameter case, or add the _validate_on_get condition below for consistency just "for now":
if self._parameter._validate_on_get:
self._parameter.validate(value)
what do you think?
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.
Actually this never matters. It calls set on the base parameter which will always do a validate. In anycase it should validate against the value and not the raw_value. So lets perhaps just remove this validator
Co-authored-by: Mikhail Astafev <[email protected]>
There are a few open questions. Especially around how we handle traits such as gettable/settable/snapshot_get being inherited from the source parameter or alternatively set specifically on the Delegated parameter