Fixes #354 bug that unitful_property with limits fails (#355)

* Fix bug that unitful_property with limits did fail on comparison with unitless value. Fixes #354.

Assume_units accepts strings as well.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Unitful_property bounds use assume unit on bounds as well.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Test adjusted, that limits functions return quantity.

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

src/instruments/util_fns.py

@@ -33,9 +33,14 @@ def assume_units(value, units):
         ``units``, depending on if ``value`` is unitful.
     :rtype: `Quantity`
     """
-    if not isinstance(value, u.Quantity):
-        value = u.Quantity(value, units)
-    return value
+    if isinstance(value, u.Quantity):
+        return value
+    elif isinstance(value, str):
+        value = u.Quantity(value)
+        if value.dimensionless:
+            return u.Quantity(value.magnitude, units)
+        return value
+    return u.Quantity(value, units)
 
 
 def setattr_expression(target, name_expr, value):
@@ -493,10 +498,13 @@ def _getter(self):
         return u.Quantity(*split_unit_str(raw, units)).to(units)
 
     def _setter(self, newval):
+        newval = assume_units(newval, units).to(units)
         min_value, max_value = valid_range
         if min_value is not None:
             if callable(min_value):
                 min_value = min_value(self)  # pylint: disable=not-callable
+            else:
+                min_value = assume_units(min_value, units)
             if newval < min_value:
                 raise ValueError(
                     f"Unitful quantity is too low. Got {newval}, "
@@ -505,14 +513,16 @@ def _setter(self, newval):
         if max_value is not None:
             if callable(max_value):
                 max_value = max_value(self)  # pylint: disable=not-callable
+            else:
+                max_value = assume_units(max_value, units)
             if newval > max_value:
                 raise ValueError(
                     f"Unitful quantity is too high. Got {newval}, "
                     f"maximum value is {max_value}"
                 )
         # Rescale to the correct unit before printing. This will also
         # catch bad units.
-        strval = format_code.format(assume_units(newval, units).to(units).magnitude)
+        strval = format_code.format(newval.magnitude)
         self.sendcmd(
             set_fmt.format(
                 command if set_cmd is None else set_cmd, _out_decor_fcn(strval)

tests/test_property_factories/test_unitful_property.py

@@ -127,10 +127,10 @@ class UnitfulMock(MockInstrument):
 def test_unitful_property_valid_range_functions():
     class UnitfulMock(MockInstrument):
         def min_value(self):
-            return 0
+            return 0 * u.Hz
 
         def max_value(self):
-            return 10
+            return 10 * u.Hz
 
         unitful_property = unitful_property(
             "MOCK", u.hertz, valid_range=(min_value, max_value)

tests/test_util_fns.py

@@ -23,6 +23,8 @@
     unitless_property,
 )
 
+from tests import unit_eq
+
 
 # FIXTURES ###################################################################
 
@@ -70,7 +72,15 @@ class SomeEnum(Enum):
 
         int_property = int_property("42")
 
-        unitful_property = unitful_property("42", u.K)
+        unitful_property_limited = unitful_property(
+            "42", u.m, valid_range=(1 * u.m, 100 * u.m)
+        )
+
+        unitful_property_limited_numbers = unitful_property(
+            "42", u.m, valid_range=(1, 100.0)
+        )
+
+        unitful_property = unitful_property("42", u.m)
 
         string_property = string_property("'STRING'")
 
@@ -178,19 +188,18 @@ def __init__(self, parent, name):
         _ = proxy_list[10]  # Should raise IndexError
 
 
-def test_assume_units_correct():
-    m = u.Quantity(1, "m")
-
-    # Check that unitful quantities are kept unitful.
-    assert assume_units(m, "mm").to("mm").magnitude == 1000
-
-    # Check that raw scalars are made unitful.
-    assert assume_units(1, "m").to("mm").magnitude == 1000
-
-
-def test_assume_units_failures():
-    with pytest.raises(pint.errors.DimensionalityError):
-        assume_units(1, "m").to("s")
+@pytest.mark.parametrize(
+    "input, out",
+    (
+        (1, u.Quantity(1, "m")),
+        (5 * u.mm, u.Quantity(5, "mm")),
+        ("7.3 km", u.Quantity(7.3, "km")),
+        ("7.5", u.Quantity(7.5, u.m)),
+        (u.Quantity(9, "nm"), 9 * u.nm),
+    ),
+)
+def test_assume_units_correct(input, out):
+    unit_eq(assume_units(input, "m"), out)
 
 
 def test_setattr_expression_simple():
@@ -289,16 +298,82 @@ def test_int_property_sendcmd_query(mock_inst):
     mock_inst.spy_sendcmd.assert_called()
 
 
-def test_unitful_property_sendcmd_query(mock_inst):
-    """Assert that unitful_property calls sendcmd, query of parent class."""
-    # getter
-    assert mock_inst.unitful_property == u.Quantity(42, u.K)
-    mock_inst.spy_query.assert_called()
-    # setter
-    value = 13
-    mock_inst.unitful_property = u.Quantity(value, u.K)
-    assert mock_inst._sendcmd == f"42 {value:e}"
-    mock_inst.spy_sendcmd.assert_called()
+class Test_unitful_property:
+    def test_unitful_property_sendcmd_query(self, mock_inst):
+        """Assert that unitful_property calls sendcmd, query of parent class."""
+        # getter
+        assert mock_inst.unitful_property == u.Quantity(42, u.m)
+        mock_inst.spy_query.assert_called()
+        # setter
+        value = 13
+        mock_inst.unitful_property = u.Quantity(value, u.m)
+        assert mock_inst._sendcmd == f"42 {value:e}"
+        mock_inst.spy_sendcmd.assert_called()
+
+    def test_unitful_property_sendcmd_query_unitless(self, mock_inst):
+        """Assert that unitful_property calls sendcmd, query of parent class.
+        Here for a unitless input
+        """
+        # getter
+        assert mock_inst.unitful_property == u.Quantity(42, u.m)
+        mock_inst.spy_query.assert_called()
+        # setter
+        value = 13
+        mock_inst.unitful_property = value
+        assert mock_inst._sendcmd == f"42 {value:e}"
+        mock_inst.spy_sendcmd.assert_called()
+
+    @pytest.mark.parametrize("value", (0.1, 200, 0.1 * u.m, 200 * u.m))
+    def test_unitful_property_sendcmd_limited_unfit(self, mock_inst, value):
+        """Assert that unitful_property calls sendcmd, query of parent class.
+        Here an input out of bounds for quantity limited property."""
+        # setter
+        with pytest.raises(ValueError):
+            mock_inst.unitful_property_limited = value
+
+    @pytest.mark.parametrize("value", (13 * u.m, 17 * u.m, 55 * u.m))
+    def test_unitful_property_sendcmd_limited_pass_un(self, mock_inst, value):
+        """Assert that unitful_property calls sendcmd, query of parent class.
+        Here a quantity input fit for quantity limited property."""
+        # setter
+        mock_inst.unitful_property_limited = value
+        assert mock_inst._sendcmd == f"42 {value.magnitude:e}"
+        mock_inst.spy_sendcmd.assert_called()
+
+    @pytest.mark.parametrize("value", (13, 17.0, 55.5, 99))
+    def test_unitful_property_sendcmd_limited_pass_ul(self, mock_inst, value):
+        """Assert that unitful_property calls sendcmd, query of parent class.
+        Here a numbers input fit for quantity limited property."""
+        # setter
+        mock_inst.unitful_property_limited = value
+        assert mock_inst._sendcmd == f"42 {value:e}"
+        mock_inst.spy_sendcmd.assert_called()
+
+    @pytest.mark.parametrize("value", (0.1, 200, 0.1 * u.m, 200 * u.m))
+    def test_unitful_property_sendcmd_limited_unfit2(self, mock_inst, value):
+        """Assert that unitful_property calls sendcmd, query of parent class.
+        Here an input out of numbered bounds for limited property."""
+        # setter
+        with pytest.raises(ValueError):
+            mock_inst.unitful_property_limited_numbers = value
+
+    @pytest.mark.parametrize("value", (13 * u.m, 17 * u.m, 55 * u.m))
+    def test_unitful_property_sendcmd_limited_pass_un2(self, mock_inst, value):
+        """Assert that unitful_property calls sendcmd, query of parent class.
+        Here a quantity input fit for numbers limited property."""
+        # setter
+        mock_inst.unitful_property_limited_numbers = value
+        assert mock_inst._sendcmd == f"42 {value.magnitude:e}"
+        mock_inst.spy_sendcmd.assert_called()
+
+    @pytest.mark.parametrize("value", (13, 17.0, 55.5, 99))
+    def test_unitful_property_sendcmd_limited_pass_ul2(self, mock_inst, value):
+        """Assert that unitful_property calls sendcmd, query of parent class.
+        Here a numbers input fit for numbers limited property."""
+        # setter
+        mock_inst.unitful_property_limited_numbers = value
+        assert mock_inst._sendcmd == f"42 {value:e}"
+        mock_inst.spy_sendcmd.assert_called()
 
 
 def test_string_property_sendcmd_query(mock_inst):