Skip to content

Add function to instrument to validate all gettable parameters #347

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.

Sign up for GitHub

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

Closed
peendebak wants to merge 3 commits into from
Closed

Add function to instrument to validate all gettable parameters #347

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3066d86
add validate_status function and basic test
eendebakpt Sep 30, 2016
d9fe662
autopep
eendebakpt Sep 30, 2016
3b0a34f
rename test function; use self.assetRaises in test
eendebakpt Oct 21, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions qcodes/instrument/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -628,3 +628,12 @@ def __getstate__(self):
'were trying to use a local instrument (defined with '
'server_name=None) in a background Loop. Local instruments can '
'only be used in Loops with background=False.')

def validate_status(self, verbose=0):
""" Validate the values of all gettable parameters """
Copy link
Contributor

@giulioungaretti giulioungaretti Oct 23, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer verbose to be a real bool, than the 0 (which suggests a level of verbosity rather than a switch. ).

And what happens if the parameter is gettable, but not settable ?

Copy link
Contributor Author

@peendebak peendebak Oct 27, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The verbose parameter is an int on purpose. In general I use the convetion: 0: no output, 1: normal output, >=2: used for debugging, output not stable

Copy link
Contributor Author

@peendebak peendebak Oct 27, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the parameter is gettable but not settable, then nothing happens. The assumption here is that if there is no .set, then there is probably no meaningfull .validate.
If we want we can remove the check on has_set, since every parameter has a .validate function.

Copy link
Contributor

@giulioungaretti giulioungaretti Oct 27, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a) Cool, just write the assumption in the docstring !

b) In this function 2 and 1 are the same then, which I don't like, and I don't see it documented or understandable from the code.
Change to true and false in this case.

Copy link
Contributor Author

@peendebak peendebak Oct 27, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@giulioungaretti Ok, changes have been done in #369 which points to

https://github.com/VandersypenQutech/Qcodes/tree/origin/feat/validate_status

for k, p in self.parameters.items():
if p.has_get and p.has_set:
value = p.get()
if verbose:
print('validate_status: param %s: %s' % (k, value))
p.validate(value)
13 changes: 11 additions & 2 deletions qcodes/tests/test_instrument.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ def setUpClass(cls):

cls.gates = MockGates(model=cls.model, server_name='')
cls.source = MockSource(model=cls.model, server_name='')
cls.meter = MockMeter(model=cls.model, keep_history=False, server_name='')
cls.meter = MockMeter(
model=cls.model, keep_history=False, server_name='')
Copy link
Contributor

@giulioungaretti giulioungaretti Oct 23, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this acutally good PEP8 ? model should be aligned with the ( , no ?

Copy link
Contributor Author

@peendebak peendebak Oct 27, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My autopep8 formatter says its fine

Copy link
Contributor

@giulioungaretti giulioungaretti Oct 27, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cool


def setUp(self):
# reset the model state via the gates function
Expand Down Expand Up @@ -963,6 +964,14 @@ def tearDown(self):
# TODO (giulioungaretti) remove ( does nothing ?)
pass

def test_validate_function(self):
instrument = self.instrument
instrument.validate_status() # test the instrument has valid values

instrument.dac1._save_val(1000) # overrule the validator
with self.assertRaises(Exception):
instrument.validate_status()

def test_attr_access(self):
instrument = self.instrument

Expand All @@ -975,7 +984,7 @@ def test_attr_access(self):
instrument.close()

# make sure we can still print the instrument
s = instrument.__repr__()
_ = instrument.__repr__()

# make sure the gate is removed
self.assertEqual(hasattr(instrument, 'dac1'), False)