Merge pull request #796 from mcdo0486/fix_results_column_units

Fix results not adding a data column that isn't in Pint unit registry

pymeasure/display/windows/managed_window.py

@@ -148,6 +148,9 @@ def __init__(self,
         # Check if the get_estimates function is reimplemented
         self.use_estimator = not self.procedure_class.get_estimates == Procedure.get_estimates
 
+        # Validate DATA_COLUMNS fit pymeasure column header format
+        Procedure.parse_columns(self.procedure_class.DATA_COLUMNS)
+
         self._setup_ui()
         self._layout()
 

pymeasure/experiment/procedure.py

@@ -27,8 +27,11 @@
 import inspect
 from copy import deepcopy
 from importlib.machinery import SourceFileLoader
+import re
+from pint import UndefinedUnitError
 
 from .parameters import Parameter, Measurable, Metadata
+from pymeasure.units import ureg
 
 log = logging.getLogger()
 log.addHandler(logging.NullHandler())
@@ -75,6 +78,33 @@ def __init__(self, **kwargs):
                 log.info(f'Setting parameter {key} to {kwargs[key]}')
         self.gen_measurement()
 
+    @staticmethod
+    def parse_columns(columns):
+        """Get columns with any units in parentheses.
+        For each column, if there are matching parentheses containing text
+        with no spaces, parse the value between the parentheses as a Pint unit. For example,
+        "Source Voltage (V)" will be parsed and matched to :code:`Unit('volt')`.
+        Raises an error if a parsed value is undefined in Pint unit registry.
+        Return a dictionary of matched columns with their units.
+
+        :param columns: List of columns to be parsed.
+        :type record: dict
+        :return: Dictionary of columns with Pint units.
+        """
+        units_pattern = r"\((?P<units>[\w/\(\)\*\t]+)\)"
+        units = {}
+        for column in columns:
+            match = re.search(units_pattern, column)
+            if match:
+                try:
+                    units[column] = ureg.Quantity(match.groupdict()['units']).units
+                except UndefinedUnitError:
+                    raise ValueError(
+                        f"Column \"{column}\" with unit \"{match.groupdict()['units']}\""
+                        " is not defined in Pint registry. Check procedure "
+                        "DATA_COLUMNS contains valid Pint units.")
+        return units
+
     def gen_measurement(self):
         """Create MEASURE and DATA_COLUMNS variables for get_datapoint method."""
         # TODO: Refactor measurable-s implementation to be consistent with parameters
@@ -88,6 +118,9 @@ def gen_measurement(self):
         if not self.DATA_COLUMNS:
             self.DATA_COLUMNS = Measurable.DATA_COLUMNS
 
+        # Validate DATA_COLUMNS fit pymeasure column header format
+        self.parse_columns(self.DATA_COLUMNS)
+
     def get_datapoint(self):
         data = {key: getattr(self, self.MEASURE[key]).value for key in self.MEASURE}
         return data

pymeasure/experiment/results.py

@@ -121,6 +121,8 @@ def unique_filename(directory, prefix='DATA', suffix='', ext='csv',
 class CSVFormatter(logging.Formatter):
     """ Formatter of data results """
 
+    numeric_types = (float, int, Decimal)
+
     def __init__(self, columns, delimiter=','):
         """Creates a csv formatter for a given list of columns (=header).
 
@@ -131,20 +133,9 @@ def __init__(self, columns, delimiter=','):
         """
         super().__init__()
         self.columns = columns
-        self.units = self._parse_columns(columns)
+        self.units = Procedure.parse_columns(columns)
         self.delimiter = delimiter
 
-    @staticmethod
-    def _parse_columns(columns):
-        """Parse the columns to get units in parenthesis."""
-        units_pattern = r"\((?P<units>[\w/\(\)\*\t]+)\)"
-        units = {}
-        for column in columns:
-            match = re.search(units_pattern, column)
-            if match:
-                units[column] = ureg.Quantity(match.groupdict()['units']).units
-        return units
-
     def format(self, record):
         """Formats a record as csv.
 
@@ -155,44 +146,45 @@ def format(self, record):
         line = []
         for x in self.columns:
             value = record.get(x, float("nan"))
-            units = self.units.get(x, None)
-            if units is not None:
-                if isinstance(value, str):
-                    try:
-                        value = ureg.Quantity(value)
-                    except pint.UndefinedUnitError:
+            if type(value) in self.numeric_types:
+                line.append(f"{value}")
+            else:
+                units = self.units.get(x, None)
+                if units is not None:
+                    if isinstance(value, str):
+                        try:
+                            value = ureg.Quantity(value)
+                        except pint.UndefinedUnitError:
+                            log.warning(
+                                f"Value {value} for column {x} cannot be parsed to"
+                                f" unit {units}.")
+                    if isinstance(value, pint.Quantity):
+                        try:
+                            line.append(f"{value.m_as(units)}")
+                        except pint.DimensionalityError:
+                            line.append("nan")
+                            log.warning(
+                                f"Value {value} for column {x} does not have the "
+                                f"right unit {units}.")
+                    elif isinstance(value, bool):
+                        line.append("nan")
                         log.warning(
-                            f"Value {value} for column {x} cannot be parsed to"
-                            f" unit {units}.")
-                if isinstance(value, pint.Quantity):
-                    try:
-                        line.append(f"{value.m_as(units)}")
-                    except pint.DimensionalityError:
+                            f"Boolean for column {x} does not have unit {units}.")
+                    else:
                         line.append("nan")
                         log.warning(
-                            f"Value {value} for column {x} does not have the "
-                            f"right unit {units}.")
-                elif isinstance(value, bool):
-                    line.append("nan")
-                    log.warning(
-                        f"Boolean for column {x} does not have unit {units}.")
-                elif isinstance(value, (float, int, Decimal)):
-                    line.append(f"{value}")
+                            f"Value {value} for column {x} does not have the right"
+                            f" type for unit {units}.")
                 else:
-                    line.append("nan")
-                    log.warning(
-                        f"Value {value} for column {x} does not have the right"
-                        f" type for unit {units}.")
-            else:
-                if isinstance(value, pint.Quantity):
-                    if value.units == ureg.dimensionless:
-                        line.append(f"{value.magnitude}")
+                    if isinstance(value, pint.Quantity):
+                        if value.units == ureg.dimensionless:
+                            line.append(f"{value.magnitude}")
+                        else:
+                            self.units[x] = value.to_base_units().units
+                            line.append(f"{value.m_as(self.units[x])}")
+                            log.info(f"Column {x} units was set to {self.units[x]}")
                     else:
-                        self.units[x] = value.to_base_units().units
-                        line.append(f"{value.m_as(self.units[x])}")
-                        log.info(f"Column {x} units was set to {self.units[x]}")
-                else:
-                    line.append(f"{value}")
+                        line.append(f"{value}")
         return self.delimiter.join(line)
 
     def format_header(self):

pymeasure/instruments/tdk/tdk_gen40_38.py

@@ -44,10 +44,10 @@ class TDK_Gen40_38(TDK_Lambda_Base):
 
         psu = TDK_Gen40_38("COM3", 6)       # COM port and daisy-chain address
         psu.remote = "REM"                  # PSU in remote mode
-        psu.source_output = "ON"            # Turn on output
+        psu.output_enabled = True           # Turn on output
         psu.ramp_to_current(2.0)            # Ramp to 2.0 A of current
-        print(psu.actual_current)           # Measure actual PSU current
-        print(psu.actual_voltage)           # Measure actual PSU voltage
+        print(psu.current)                  # Measure actual PSU current
+        print(psu.voltage)                  # Measure actual PSU voltage
         psu.shutdown()                      # Run shutdown command
 
     The initialization of a TDK instrument requires the current address

pymeasure/instruments/tdk/tdk_gen80_65.py

@@ -44,10 +44,10 @@ class TDK_Gen80_65(TDK_Lambda_Base):
 
         psu = TDK_Gen80_65("COM3", 6)       # COM port and daisy-chain address
         psu.remote = "REM"                  # PSU in remote mode
-        psu.source_output = "ON"            # Turn on output
+        psu.output_enabled = True           # Turn on output
         psu.ramp_to_current(2.0)            # Ramp to 2.0 A of current
-        print(psu.actual_current)           # Measure actual PSU current
-        print(psu.actual_voltage)           # Measure actual PSU voltage
+        print(psu.current)                  # Measure actual PSU current
+        print(psu.voltage)                  # Measure actual PSU voltage
         psu.shutdown()                      # Run shutdown command
 
     The initialization of a TDK instrument requires the current address

tests/experiment/test_procedure.py

@@ -27,6 +27,7 @@
 
 from pymeasure.experiment.procedure import Procedure, ProcedureWrapper
 from pymeasure.experiment.parameters import Parameter
+from pymeasure.units import ureg
 
 from data.procedure_for_testing import RandomProcedure
 
@@ -44,6 +45,7 @@ class TestProcedure(Procedure):
     assert 'x' in objs
     assert objs['x'].value == p.x
 
+
 # TODO: Add tests for measureables
 
 
@@ -58,6 +60,7 @@ def test_procedure_wrapper():
     assert new_wrapper.procedure.iterations == 101
     assert RandomProcedure.iterations.value == 100
 
+
 # This test checks that user can define properties using the parameters inside the procedure
 # The test ensure that property is evaluated only when the Parameter has been processed during
 # class initialization.
@@ -74,11 +77,13 @@ def a(self):
         def z(self):
             assert isinstance(self.x, int)
             return self.x
+
         x = Parameter('X', default=5)
 
     p = TestProcedure()
     assert p.x == 5
 
+
 # Make sure that a procedure can be initialized even though some properties are raising
 # errors at initialization time
 
@@ -88,8 +93,34 @@ class TestProcedure(Procedure):
         @property
         def prop(self):
             return self.x
+
     p = TestProcedure()
     with pytest.raises(AttributeError):
         _ = p.prop  # AttributeError
     p.x = 5
     assert p.prop == 5
+
+
+@pytest.mark.parametrize("header, units", (
+        ("x (m)", ureg.m),
+        ("x (m/s)", ureg.m / ureg.s),
+        ("x (V/(m*s))", ureg.V / ureg.m / ureg.s),
+        ("x (1)", ureg.dimensionless)
+))
+def test_procedure_parse_columns(header, units):
+    assert Procedure.parse_columns([header])[header] == ureg.Quantity(1, units)
+
+
+@pytest.mark.parametrize("valid_header_no_unit", (
+        ["x"], ["x ( x + y )"], ["x ( notes )"], ["x [V]"]
+))
+def test_procedure_no_parsed_units(valid_header_no_unit):
+    assert Procedure.parse_columns(valid_header_no_unit) == {}
+
+
+@pytest.mark.parametrize("invalid_header_unit", (
+        ["x (sqrt)"], ["x (x)"], ["x (y)"],
+))
+def test_procedure_invalid_parsed_unit(invalid_header_unit):
+    with pytest.raises(ValueError):
+        Procedure.parse_columns(invalid_header_unit)

tests/experiment/test_results.py

@@ -96,15 +96,6 @@ def test_unitful_erroneous(self):
         assert formatter.format(data) == "nan,nan,nan"
 
 
-@pytest.mark.parametrize("header, units", (
-    ("x (m)", ureg.m),
-    ("x (m/s)", ureg.m/ureg.s),
-    ("x (V/(m*s))", ureg.V / ureg.m / ureg.s),
-    ))
-def test_csv_formatter_parse_columns(header, units):
-    assert CSVFormatter._parse_columns([header])[header] == ureg.Quantity(1, units)
-
-
 def test_procedure_filestorage():
     assert RandomProcedure.iterations.value == 100
     procedure = RandomProcedure()