From 6cfe46db537813f5ae480ce07e7c161ad0d182fa Mon Sep 17 00:00:00 2001 From: Giulio Ungaretti Date: Wed, 3 Aug 2016 16:21:57 +0200 Subject: [PATCH 1/2] refactor: Move parameter test inot own file --- qcodes/tests/test_instrument.py | 148 ++----------------------------- qcodes/tests/test_parameter.py | 151 ++++++++++++++++++++++++++++++++ 2 files changed, 157 insertions(+), 142 deletions(-) create mode 100644 qcodes/tests/test_parameter.py diff --git a/qcodes/tests/test_instrument.py b/qcodes/tests/test_instrument.py index 98d7b3a641ea..77069cf986bb 100644 --- a/qcodes/tests/test_instrument.py +++ b/qcodes/tests/test_instrument.py @@ -1,13 +1,13 @@ -from unittest import TestCase +""" +Test suite for instument.* +""" from datetime import datetime, timedelta +from unittest import TestCase import time -from collections import namedtuple from qcodes.instrument.base import Instrument from qcodes.instrument.mock import MockInstrument -from qcodes.instrument.parameter import (Parameter, ManualParameter, - StandardParameter) -from qcodes.instrument.function import Function +from qcodes.instrument.parameter import ManualParameter from qcodes.instrument.server import get_instrument_server_manager from qcodes.utils.validators import Numbers, Ints, Strings, MultiType, Enum @@ -20,112 +20,6 @@ from .common import strip_qc -class TestParamConstructor(TestCase): - - def test_name_s(self): - p = Parameter('simple') - self.assertEqual(p.name, 'simple') - - with self.assertRaises(ValueError): - # you need a name of some sort - Parameter() - - # or names - names = ['H1', 'L1'] - p = Parameter(names=names) - self.assertEqual(p.names, names) - # if you don't provide a name, it's called 'None' - # TODO: we should probably require an explicit name. - self.assertEqual(p.name, 'None') - - # or both, that's OK too. - names = ['Peter', 'Paul', 'Mary'] - p = Parameter(name='complex', names=names) - self.assertEqual(p.names, names) - # You can always have a name along with names - self.assertEqual(p.name, 'complex') - - shape = (10,) - setpoints = (range(10),) - setpoint_names = ('my_sp',) - setpoint_labels = ('A label!',) - p = Parameter('makes_array', shape=shape, setpoints=setpoints, - setpoint_names=setpoint_names, - setpoint_labels=setpoint_labels) - self.assertEqual(p.shape, shape) - self.assertFalse(hasattr(p, 'shapes')) - self.assertEqual(p.setpoints, setpoints) - self.assertEqual(p.setpoint_names, setpoint_names) - self.assertEqual(p.setpoint_labels, setpoint_labels) - - shapes = ((2,), (3,)) - setpoints = ((range(2),), (range(3),)) - setpoint_names = (('sp1',), ('sp2',)) - setpoint_labels = (('first label',), ('second label',)) - p = Parameter('makes arrays', shapes=shapes, setpoints=setpoints, - setpoint_names=setpoint_names, - setpoint_labels=setpoint_labels) - self.assertEqual(p.shapes, shapes) - self.assertFalse(hasattr(p, 'shape')) - self.assertEqual(p.setpoints, setpoints) - self.assertEqual(p.setpoint_names, setpoint_names) - self.assertEqual(p.setpoint_labels, setpoint_labels) - - def test_repr(self): - for i in [0, "foo", "", "fåil"]: - with self.subTest(i=i): - param = Parameter(name=i) - s = param.__repr__() - st = '<{}.{}: {} at {}>'.format( - param.__module__, param.__class__.__name__, - param.name, id(param)) - self.assertEqual(s, st) - - blank_instruments = ( - None, # no instrument at all - namedtuple('noname', '')(), # no .name - namedtuple('blank', 'name')('') # blank .name - ) - named_instrument = namedtuple('yesname', 'name')('astro') - - def test_full_name(self): - # three cases where only name gets used for full_name - for instrument in self.blank_instruments: - p = Parameter(name='fred') - p._instrument = instrument - self.assertEqual(p.full_name, 'fred') - - p.name = None - self.assertEqual(p.full_name, None) - - # and finally an instrument that really has a name - p = Parameter(name='wilma') - p._instrument = self.named_instrument - self.assertEqual(p.full_name, 'astro_wilma') - - p.name = None - self.assertEqual(p.full_name, None) - - def test_full_names(self): - for instrument in self.blank_instruments: - # no instrument - p = Parameter(name='simple') - p._instrument = instrument - self.assertEqual(p.full_names, None) - - p = Parameter(names=['a', 'b']) - p._instrument = instrument - self.assertEqual(p.full_names, ['a', 'b']) - - p = Parameter(name='simple') - p._instrument = self.named_instrument - self.assertEqual(p.full_names, None) - - p = Parameter(names=['penn', 'teller']) - p._instrument = self.named_instrument - self.assertEqual(p.full_names, ['astro_penn', 'astro_teller']) - - class GatesBadDelayType(MockGates): def __init__(self, *args, **kwargs): @@ -584,37 +478,6 @@ def test_val_mapping_parsers(self): with self.assertRaises(ValueError): gates.modecoded.set('0') - def test_param_cmd_with_parsing(self): - def set_p(val): - self._p = val - - def get_p(): - return self._p - - def parse_set_p(val): - return '{:d}'.format(val) - - p = StandardParameter('p_int', get_cmd=get_p, get_parser=int, - set_cmd=set_p, set_parser=parse_set_p) - - p(5) - self.assertEqual(self._p, '5') - self.assertEqual(p(), 5) - - def test_bare_function(self): - # not a use case we want to promote, but it's there... - p = ManualParameter('test') - - def doubler(x): - p.set(x * 2) - - f = Function('f', call_cmd=doubler, args=[Numbers(-10, 10)]) - - f(4) - self.assertEqual(p.get(), 8) - with self.assertRaises(ValueError): - f(20) - def test_standard_snapshot(self): self.maxDiff = None snap = self.meter.snapshot() @@ -997,6 +860,7 @@ def setUp(self): name='testdummy', gates=['dac1', 'dac2', 'dac3'], server_name=None) def tearDown(self): + #TODO (giulioungaretti) remove ( does nothing ?) pass def test_attr_access(self): diff --git a/qcodes/tests/test_parameter.py b/qcodes/tests/test_parameter.py new file mode 100644 index 000000000000..73ed3d07bad8 --- /dev/null +++ b/qcodes/tests/test_parameter.py @@ -0,0 +1,151 @@ +""" +Test suite for parameter +""" +from collections import namedtuple +from unittest import TestCase + +from qcodes import Function +from qcodes.instrument.parameter import (Parameter, ManualParameter, + StandardParameter) +from qcodes.utils.validators import Numbers + + +class TestParamConstructor(TestCase): + + def test_name_s(self): + p = Parameter('simple') + self.assertEqual(p.name, 'simple') + + with self.assertRaises(ValueError): + # you need a name of some sort + Parameter() + + # or names + names = ['H1', 'L1'] + p = Parameter(names=names) + self.assertEqual(p.names, names) + # if you don't provide a name, it's called 'None' + # TODO: we should probably require an explicit name. + self.assertEqual(p.name, 'None') + + # or both, that's OK too. + names = ['Peter', 'Paul', 'Mary'] + p = Parameter(name='complex', names=names) + self.assertEqual(p.names, names) + # You can always have a name along with names + self.assertEqual(p.name, 'complex') + + shape = (10,) + setpoints = (range(10),) + setpoint_names = ('my_sp',) + setpoint_labels = ('A label!',) + p = Parameter('makes_array', shape=shape, setpoints=setpoints, + setpoint_names=setpoint_names, + setpoint_labels=setpoint_labels) + self.assertEqual(p.shape, shape) + self.assertFalse(hasattr(p, 'shapes')) + self.assertEqual(p.setpoints, setpoints) + self.assertEqual(p.setpoint_names, setpoint_names) + self.assertEqual(p.setpoint_labels, setpoint_labels) + + shapes = ((2,), (3,)) + setpoints = ((range(2),), (range(3),)) + setpoint_names = (('sp1',), ('sp2',)) + setpoint_labels = (('first label',), ('second label',)) + p = Parameter('makes arrays', shapes=shapes, setpoints=setpoints, + setpoint_names=setpoint_names, + setpoint_labels=setpoint_labels) + self.assertEqual(p.shapes, shapes) + self.assertFalse(hasattr(p, 'shape')) + self.assertEqual(p.setpoints, setpoints) + self.assertEqual(p.setpoint_names, setpoint_names) + self.assertEqual(p.setpoint_labels, setpoint_labels) + + def test_repr(self): + for i in [0, "foo", "", "fåil"]: + with self.subTest(i=i): + param = Parameter(name=i) + s = param.__repr__() + st = '<{}.{}: {} at {}>'.format( + param.__module__, param.__class__.__name__, + param.name, id(param)) + self.assertEqual(s, st) + + blank_instruments = ( + None, # no instrument at all + namedtuple('noname', '')(), # no .name + namedtuple('blank', 'name')('') # blank .name + ) + named_instrument = namedtuple('yesname', 'name')('astro') + + def test_full_name(self): + # three cases where only name gets used for full_name + for instrument in self.blank_instruments: + p = Parameter(name='fred') + p._instrument = instrument + self.assertEqual(p.full_name, 'fred') + + p.name = None + self.assertEqual(p.full_name, None) + + # and finally an instrument that really has a name + p = Parameter(name='wilma') + p._instrument = self.named_instrument + self.assertEqual(p.full_name, 'astro_wilma') + + p.name = None + self.assertEqual(p.full_name, None) + + def test_full_names(self): + for instrument in self.blank_instruments: + # no instrument + p = Parameter(name='simple') + p._instrument = instrument + self.assertEqual(p.full_names, None) + + p = Parameter(names=['a', 'b']) + p._instrument = instrument + self.assertEqual(p.full_names, ['a', 'b']) + + p = Parameter(name='simple') + p._instrument = self.named_instrument + self.assertEqual(p.full_names, None) + + p = Parameter(names=['penn', 'teller']) + p._instrument = self.named_instrument + self.assertEqual(p.full_names, ['astro_penn', 'astro_teller']) + +class TestManualParameter(TestCase): + + def test_bare_function(self): + # not a use case we want to promote, but it's there... + p = ManualParameter('test') + + def doubler(x): + p.set(x * 2) + + f = Function('f', call_cmd=doubler, args=[Numbers(-10, 10)]) + + f(4) + self.assertEqual(p.get(), 8) + with self.assertRaises(ValueError): + f(20) + +class TestStandardParam(TestCase): + + def test_param_cmd_with_parsing(self): + def set_p(val): + self._p = val + + def get_p(): + return self._p + + def parse_set_p(val): + return '{:d}'.format(val) + + p = StandardParameter('p_int', get_cmd=get_p, get_parser=int, + set_cmd=set_p, set_parser=parse_set_p) + + p(5) + self.assertEqual(self._p, '5') + self.assertEqual(p(), 5) \ No newline at end of file From 494acda0ca2433bb2d4977c3a1ba71b8d21d7a18 Mon Sep 17 00:00:00 2001 From: Giulio Ungaretti Date: Thu, 4 Aug 2016 10:53:28 +0200 Subject: [PATCH 2/2] fix: Eradicate bad practice. --- qcodes/tests/instrument_mocks.py | 27 ++++++++++++----------- qcodes/utils/helpers.py | 37 ++++++++++++++++---------------- 2 files changed, 33 insertions(+), 31 deletions(-) diff --git a/qcodes/tests/instrument_mocks.py b/qcodes/tests/instrument_mocks.py index 9e7fa7d8180e..685b5ed380bc 100644 --- a/qcodes/tests/instrument_mocks.py +++ b/qcodes/tests/instrument_mocks.py @@ -1,6 +1,4 @@ import time -import logging -from functools import partial from qcodes.instrument.base import Instrument from qcodes.instrument.mock import MockInstrument, MockModel @@ -19,7 +17,8 @@ def _reset(self): self._gates = [0.0, 0.0, 0.0] self._excitation = 0.1 - def fmt(self, value): + @staticmethod + def fmt(value): return '{:.3f}'.format(value) def gates_set(self, parameter, value): @@ -48,6 +47,7 @@ def source_set(self, parameter, value): if parameter == 'ampl': try: self._excitation = float(value) + # TODO(giulioungaretti) fix bare-except except: # "Off" as in the MultiType sweep step test self._excitation = None @@ -91,25 +91,25 @@ def __init__(self, *args, **kwargs): self.attach_adder() def attach_adder(self): - ''' + """ this function attaches a closure to the object, so can only be executed after creating the server because a closure is not picklable - ''' + """ a = 5 def f(b): - ''' + """ not the same function as the original method - ''' + """ return a + b self.add5 = f def add5(self, b): - ''' + """ The class copy of this should not get run, because it should be overwritten on the server by the closure version. - ''' + """ raise RuntimeError('dont run this one!') @@ -186,17 +186,18 @@ def __init__(self, model=None, **kwargs): class DummyInstrument(Instrument): def __init__(self, name='dummy', gates=['dac1', 'dac2', 'dac3'], **kwargs): - ''' Create a dummy instrument that can be used for testing - + """ + Create a dummy instrument that can be used for testing + Args: name (string): name for the instrument gates (list): list of names that is used to create parameters for the instrument - ''' + """ super().__init__(name, **kwargs) # make gates - for i, g in enumerate(gates): + for _, g in enumerate(gates): self.add_parameter(g, parameter_class=ManualParameter, initial_value=0, diff --git a/qcodes/utils/helpers.py b/qcodes/utils/helpers.py index 5aee66d5f07c..bf93e6b17dfb 100644 --- a/qcodes/utils/helpers.py +++ b/qcodes/utils/helpers.py @@ -1,23 +1,23 @@ -from collections import Iterator, Sequence, Mapping -from copy import deepcopy -import time +import io +import json import logging import math +import numbers import sys -import io +import time + +from collections import Iterator, Sequence, Mapping +from copy import deepcopy + import numpy as np -import json -import numbers _tprint_times = {} class NumpyJSONEncoder(json.JSONEncoder): - """Return numpy types as standard types.""" # http://stackoverflow.com/questions/27050108/convert-numpy-type-to-python # http://stackoverflow.com/questions/9452775/converting-numpy-dtypes-to-native-python-types/11389998#11389998 - def default(self, obj): if isinstance(obj, np.integer): return int(obj) @@ -121,7 +121,7 @@ def deep_update(dest, update): # a) we don't want to require that as a dep so low level # b) I'd like to be more flexible with the sign of step def permissive_range(start, stop, step): - ''' + """ returns range (as a list of values) with floating point step inputs: @@ -129,7 +129,7 @@ def permissive_range(start, stop, step): always starts at start and moves toward stop, regardless of the sign of step - ''' + """ signed_step = abs(step) * (1 if stop > start else -1) # take off a tiny bit for rounding errors step_count = math.ceil((stop - start) / signed_step - 1e-10) @@ -189,11 +189,11 @@ def make_sweep(start, stop, step=None, num=None): def wait_secs(finish_clock): - ''' + """ calculate the number of seconds until a given clock time The clock time should be the result of time.perf_counter() Does NOT wait for this time. - ''' + """ delay = finish_clock - time.perf_counter() if delay < 0: logging.warning('negative delay {:.6f} sec'.format(delay)) @@ -203,7 +203,7 @@ def wait_secs(finish_clock): class LogCapture(): - ''' + """ context manager to grab all log messages, optionally from a specific logger @@ -212,7 +212,7 @@ class LogCapture(): with LogCapture() as logs: code_that_makes_logs(...) log_str = logs.value - ''' + """ def __init__(self, logger=logging.getLogger()): self.logger = logger @@ -231,10 +231,10 @@ def __exit__(self, type, value, tb): def make_unique(s, existing): - ''' + """ make string s unique, able to be added to a sequence `existing` of existing names without duplication, by appending _ to it if needed - ''' + """ n = 1 s_out = s existing = set(existing) @@ -247,7 +247,6 @@ def make_unique(s, existing): class DelegateAttributes: - """ Mixin class to create attributes of this object by delegating them to one or more dicts and/or objects @@ -256,7 +255,7 @@ class DelegateAttributes: in dir() and autocomplete - Attributes: + Attributes: delegate_attr_dicts (list): a list of names (strings) of dictionaries which are (or will be) attributes of self, whose keys should be treated as attributes of self @@ -342,7 +341,9 @@ def strip_attrs(obj, whitelist=()): for key in lst: try: del obj.__dict__[key] + # TODO (giulioungaretti) fix bare-except except: pass + # TODO (giulioungaretti) fix bare-except except: pass