From 0529ec9b5e6d4ec106b96922daa89b6f71f88f6d Mon Sep 17 00:00:00 2001
From: Richard Bowman <richard.bowman@cantab.net>
Date: Tue, 5 May 2026 23:31:35 +0100
Subject: [PATCH 1/9] First implementation of @lt.on_set

This adds basic side-effects to properties, along with some tests.
---
 src/labthings_fastapi/__init__.py   |   3 +-
 src/labthings_fastapi/properties.py | 111 +++++++++++++++++++++++++++-
 tests/test_property.py              |  60 +++++++++++++++
 3 files changed, 170 insertions(+), 4 deletions(-)

diff --git a/src/labthings_fastapi/__init__.py b/src/labthings_fastapi/__init__.py
index 4dad40c5..10cc841c 100644
--- a/src/labthings_fastapi/__init__.py
+++ b/src/labthings_fastapi/__init__.py
@@ -28,7 +28,7 @@
 from .thing_slots import thing_slot
 from .thing_server_interface import ThingServerInterface
 from .thing_class_settings import ThingClassSettings
-from .properties import property, setting, DataProperty, DataSetting
+from .properties import property, setting, on_set, DataProperty, DataSetting
 from .actions import action
 from .endpoints import endpoint
 from . import outputs
@@ -54,6 +54,7 @@
     "ThingClassSettings",
     "property",
     "setting",
+    "on_set",
     "DataProperty",
     "DataSetting",
     "action",
diff --git a/src/labthings_fastapi/properties.py b/src/labthings_fastapi/properties.py
index a81f157b..9ee9d9ff 100644
--- a/src/labthings_fastapi/properties.py
+++ b/src/labthings_fastapi/properties.py
@@ -48,6 +48,7 @@ class attribute. Documentation is in strings immediately following the
 from __future__ import annotations
 import builtins
 from collections.abc import Mapping
+from functools import partial
 from types import EllipsisType
 from typing import (
     Annotated,
@@ -741,6 +742,13 @@ def __init__(
         )
         self.readonly = readonly
 
+    on_set_func: Callable[[Owner, Value], Value] | None = None
+    """A function that is called when the property is set.
+
+    This function must return the new value of the property. If it raises
+    an exception, the property's value will not change.
+    """
+
     def instance_get(self, obj: Owner) -> Value:
         """Return the property's value.
 
@@ -775,9 +783,10 @@ def __set__(
         ):
             if get_validate_properties_on_set(obj.__class__):
                 property_info = self.descriptor_info(obj)
-                obj.__dict__[self.name] = property_info.validate(value)
-            else:
-                obj.__dict__[self.name] = value
+                value = property_info.validate(value)
+            if self.on_set_func:
+                value = self.on_set_func(obj, value)
+            obj.__dict__[self.name] = value
             if emit_changed_event:
                 self.emit_changed_event(obj, value)
 
@@ -853,6 +862,102 @@ async def emit_changed_event_async(self, obj: Thing, value: Value) -> None:
             )
 
 
+def on_set(
+    property_name: str,
+) -> Callable[[Callable[[Owner, Value], Value]], OnSetDescriptor[Owner, Value]]:
+    """Run a function when a data property is set.
+
+    This decorator causes a method to be called whenever a property
+    is set. The method must return the value (and may modify it), but
+    is not responsible for "remembering" the value: that's done by
+    the data property.
+
+    If the method raises an exception, the property will not change
+    its value, and the error will propagate.
+
+    Side effects should be brief: they are performed synchronously
+    during HTTP request handling, so should not exceed a fraction
+    of a second.
+
+    :param property_name: the name of the property to which we are
+        attaching a side effect.
+    :return: a descriptor object that will attach the method to the
+        property, once the class is fully defined.
+    """
+
+    def decorator(
+        func: Callable[[Owner, Value], Value],
+    ) -> OnSetDescriptor[Owner, Value]:
+        return OnSetDescriptor(property_name=property_name, func=func)
+
+    return decorator
+
+
+class OnSetDescriptor(Generic[Owner, Value]):
+    """A class to add side effects to data properties."""
+
+    def __init__(
+        self, property_name: str, func: Callable[[Owner, Value], Value]
+    ) -> None:
+        """Initialise an OnSetDescriptor.
+
+        :param property_name: the name of the property we're attaching a side-effect to.
+        :param func: the function to run when the property is set.
+        """
+        super().__init__()
+        self.property_name = property_name
+        self.func = func
+
+    def __set_name__(self, owner: type[Owner], name: str) -> None:
+        """Attach the function to the property.
+
+        ``__set_name__`` is part of the Descriptor protocol, and is where we
+        are notified of the owning class and our name.
+
+        :param owner: the class on which we are defined.
+        :param name: the name to which this descriptor is assigned.
+        :raises AttributeError: if the specified property name is missing,
+            not a data property, assigned to multiple times, or overwritten by
+            this descriptor.
+        """
+        if self.property_name == name:
+            msg = f"On-set function '{name}' overwrites its property: rename it."
+            raise AttributeError(msg)
+        prop = getattr(owner, self.property_name, None)
+        if not isinstance(prop, DataProperty):
+            msg = "On-set functions may only be attached to data properties. "
+            msg += f"'{self.property_name}' is not a data property"
+            raise AttributeError(msg)
+        if prop.on_set_func is not None:
+            raise AttributeError(f"'{self.property_name}.on_set' has already been set.")
+        prop.on_set_func = self.func
+
+    @overload
+    def __get__(self, obj: Owner) -> Callable[[Value], Value]: ...
+
+    @overload
+    def __get__(
+        self, obj: None, type: type[Owner]
+    ) -> Callable[[Owner, Value], Value]: ...
+
+    def __get__(
+        self, obj: Owner | None, type: type[Owner] | None = None
+    ) -> Callable[[Owner, Value], Value] | Callable[[Value], Value]:
+        """Return the function.
+
+        As for regular methods, we return the function if accessed on the class, and
+        a bound version if accessed on an instance.
+
+        :param obj: the instance, if accessed on an instance.
+        :param type: the class, if accessed on a class.
+        :return: the function, or a partial object binding the function to the object.
+        """
+        if obj is None:
+            return self.func
+        else:
+            return partial(self.func, obj)
+
+
 class FunctionalProperty(BaseProperty[Owner, Value], Generic[Owner, Value]):
     """A property that uses a getter and a setter.
 
diff --git a/tests/test_property.py b/tests/test_property.py
index a778700c..44f17772 100644
--- a/tests/test_property.py
+++ b/tests/test_property.py
@@ -693,3 +693,63 @@ def myprop(self) -> int:
             @myprop.resetter
             def myprop(self) -> None:
                 pass
+
+
+def test_on_set():
+    """Test that `on_set` works as expected."""
+
+    class Example(lt.Thing):
+        intprop: int = lt.property(default=0)
+
+        shadow: int = lt.property(default=0)
+
+        @lt.on_set("intprop")
+        def _on_set_intprop(self, val: int) -> int:
+            """A function to run when intprop is set."""
+            if val < 0:
+                raise ValueError("Can't be negative.")
+            self.shadow = val
+            return val
+
+    thing = create_thing_without_server(Example)
+    assert thing.shadow == 0
+    thing.intprop = 42
+    assert thing.shadow == 42
+    with pytest.raises(ValueError, match="Can't be negative"):
+        thing.intprop = -1
+
+
+def test_bad_on_set_definitions():
+    """Test that helpful errors are raise if `on_set` is used incorrectly."""
+    with raises_or_is_caused_by(AttributeError) as excinfo:
+
+        class Example2(lt.Thing):
+            @lt.on_set("missing")
+            def set_missing(self, value):
+                return value
+
+    assert "'missing' is not a data property" in str(excinfo)
+
+    with raises_or_is_caused_by(AttributeError) as excinfo:
+
+        class Example3(lt.Thing):
+            @lt.on_set("myprop")
+            def myprop(self, value):
+                return value
+
+    assert "On-set function 'myprop' overwrites its property" in str(excinfo)
+
+    with raises_or_is_caused_by(AttributeError) as excinfo:
+
+        class Example4(lt.Thing):
+            intprop: int = lt.property(default=0)
+
+            @lt.on_set("intprop")
+            def set_intprop(self, value):
+                return value
+
+            @lt.on_set("intprop")
+            def set_intprop2(self, value):
+                return value
+
+    assert "'intprop.on_set' has already been set" in str(excinfo)

From 832c3eaf54c97c3e3efda4af4d690b78495faa0c Mon Sep 17 00:00:00 2001
From: Richard Bowman <richard.bowman@cantab.net>
Date: Wed, 6 May 2026 09:28:34 +0100
Subject: [PATCH 2/9] Run `on_set` before validating.

Running the `on_set` function before validating the property (if enabled) has a few advantages:
* If the user has forgotten to return a value, it's more likely to catch the `None` if it's invalid.
* It allows user code
---
 src/labthings_fastapi/properties.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/labthings_fastapi/properties.py b/src/labthings_fastapi/properties.py
index 9ee9d9ff..d703e10a 100644
--- a/src/labthings_fastapi/properties.py
+++ b/src/labthings_fastapi/properties.py
@@ -781,11 +781,11 @@ def __set__(
         with obj._thing_server_interface._optionally_hold_global_lock(
             self.use_global_lock
         ):
+            if self.on_set_func:
+                value = self.on_set_func(obj, value)
             if get_validate_properties_on_set(obj.__class__):
                 property_info = self.descriptor_info(obj)
                 value = property_info.validate(value)
-            if self.on_set_func:
-                value = self.on_set_func(obj, value)
             obj.__dict__[self.name] = value
             if emit_changed_event:
                 self.emit_changed_event(obj, value)

From 93a3671626ced0802d1c6581f3a61db739b4ec65 Mon Sep 17 00:00:00 2001
From: Richard Bowman <richard.bowman@cantab.net>
Date: Tue, 26 May 2026 14:07:42 +0100
Subject: [PATCH 3/9] Add documentation

This adds `on_set` to the public API docs and to the conceptual page
on properties.

It also tidies up a couple of decorators in the public API docs.
---
 docs/source/properties.rst          | 33 ++++++++++++++++++++++++++
 docs/source/public_api.rst          | 36 +++++++++++++++++++++++++----
 src/labthings_fastapi/properties.py | 11 ++++++++-
 3 files changed, 74 insertions(+), 6 deletions(-)

diff --git a/docs/source/properties.rst b/docs/source/properties.rst
index 3abbf9fc..987e5303 100644
--- a/docs/source/properties.rst
+++ b/docs/source/properties.rst
@@ -56,6 +56,39 @@ The example above will have its default value set to the empty list, as that's w
 
 Data properties may be *observed*, which means notifications will be sent when the property is written to (see below).
 
+.. _properties_on_set:
+
+Data properties with setters
+----------------------------
+
+It is possible to add a method that will be called each time a data property (or setting) is set. This may be useful in several situations:
+
+* You want to do some validation or coercion that's not done by the type hint and constraints.
+* There should be a side-effect of setting the property, like updating a setting on some hardware.
+
+To do this, you should use the `lt.on_set` decorator as shown below:
+
+.. code-block:: python
+
+    class MyThing(lt.Thing):
+        my_property: int = lt.property(default=42, readonly=True)
+        """A property that holds an integer value."""
+
+        @lt.on_set("my_property")
+        def _my_property_was_set(self, value: int):
+            """Take action because my_property was set."""
+            self._hardware.set_my_property(value)
+            return value
+
+There are a few important points to note when using `lt.on_set` in your code:
+
+* Your function *must* return a value, which will be used as the property's value. This allows `lt.on_set` to coerce values to valid ones.
+* If your function raises an exception, the value will *not* be set, and the property will keep its previous value. This allows invalid values to be rejected.
+* Your function will run every time the property is set, meaning it should complete quickly. If this function takes longer than a second, it is likely to cause HTTP timeouts.
+* It's ok to communicate with hardware, but you are likely to need to acquire any locks you need manually.
+* If global locking is enabled, the global lock will already have been acquired when your function is run: there's no need to acquire it again.
+* You do not need to "remember" the value as you would for a regular Python property - the data property already takes care of that.
+
 Functional properties
 -------------------------
 
diff --git a/docs/source/public_api.rst b/docs/source/public_api.rst
index 2b555507..e607321d 100644
--- a/docs/source/public_api.rst
+++ b/docs/source/public_api.rst
@@ -90,9 +90,9 @@ This page summarises the parts of the LabThings API that should be most frequent
     .. automethod:: labthings_fastapi.thing.Thing.get_current_invocation_logs
         :no-index:
 
-.. py:function:: property(getter: Callable[[Owner], Value]) -> FunctionalProperty[Owner, Value]
-                 property(*, default: Value, readonly: bool = False, use_global_lock: bool | None = None, **constraints: Any) -> Value
+.. py:function:: property(*, default: Value, readonly: bool = False, use_global_lock: bool | None = None, **constraints: Any) -> Value
                  property(*, default_factory: Callable[[], Value], readonly: bool = False,  use_global_lock: bool | None = None, **constraints: Any) -> Value
+                 @lt.property
 
     This function may be used to define :ref:`properties` either by decorating a function, or marking an attribute. Full documentation is available at `labthings_fastapi.properties.property` and a more in-depth discussion is available at :ref:`properties`\ . This page focuses on the most frequently used examples.
 
@@ -142,12 +142,37 @@ This page summarises the parts of the LabThings API that should be most frequent
     For a full listing of attributes that may be modified, see `DataProperty`\ .
 
 
-.. py:function:: setting(getter: Callable[[Owner], Value]) -> FunctionalSetting[Owner, Value]
-                 setting(*, default: Value, readonly: bool = False, use_global_lock: bool | None = None,  **constraints: Any) -> Value
+.. py:function:: setting(default: Value, readonly: bool = False, use_global_lock: bool | None = None,  **constraints: Any) -> Value
                  setting(*, default_factory: Callable[[], Value], readonly: bool = False, use_global_lock: bool | None = None,  **constraints: Any) -> Value
+                 @lt.setting
 
     A setting is a property that is saved to disk. It is defined in the same way as `property` but will be synchronised with the `Thing`\ 's settings file. Full documentation is available at `labthings_fastapi.properties.setting`
 
+.. py:decorator:: on_set(property_name: str)
+
+    Decorate a method to run when a data `~lt.property` is set.
+
+    This decorator causes a method to be called whenever a property
+    is set. The method must return the value (and may modify it), but
+    is not responsible for "remembering" the value: that's done by
+    the data property.
+
+    `on_set` methods should have only ``self`` and the property value as arguments,
+    and must either return a valid value for the property, or raise an exception.
+    They are intended to allow validation and coercion of values, as well as
+    allowing synchronisation, for example synchronising the value of a setting with
+    a piece of hardware.
+
+    If the method raises an exception, the property will not change
+    its value, and the error will propagate.
+
+    Side effects should be brief: they are performed synchronously
+    during HTTP request handling, so should not exceed a fraction
+    of a second. This is similar to the constraint on functional property setters:
+    anything likely to take a long time should be done in an action instead.
+
+    :param property_name: the name of the property to which we are
+        attaching a side effect.
 
 .. py:decorator:: action
                   action(use_global_lock: bool | None = None, **kwargs: Any)
@@ -277,7 +302,7 @@ This page summarises the parts of the LabThings API that should be most frequent
         :param \**kwargs: additional keyword arguments are passed to `ThingServerConfig`\ .
 
     .. py:property:: things
-      :type: collections.abc.Mapping[str, Thing]
+      :type: collections.abc.Mapping[str, lt.Thing]
 
       A read-only mapping of names to `~lt.Thing` instances, for every `~lt.Thing` attached to the server.
 
@@ -333,6 +358,7 @@ This page summarises the parts of the LabThings API that should be most frequent
         :no-index:
 
    .. py:property:: global_lock
+
         :type GlobalLock | None:
 
         A global lock object that is used to restrict concurrent execution of actions and setting of properties.
diff --git a/src/labthings_fastapi/properties.py b/src/labthings_fastapi/properties.py
index d703e10a..dbee62ec 100644
--- a/src/labthings_fastapi/properties.py
+++ b/src/labthings_fastapi/properties.py
@@ -867,17 +867,26 @@ def on_set(
 ) -> Callable[[Callable[[Owner, Value], Value]], OnSetDescriptor[Owner, Value]]:
     """Run a function when a data property is set.
 
+    See the description at :ref:`properties_on_set` for an example.
+
     This decorator causes a method to be called whenever a property
     is set. The method must return the value (and may modify it), but
     is not responsible for "remembering" the value: that's done by
     the data property.
 
+    `on_set` methods should have only ``self`` and the property value as arguments,
+    and must either return a valid value for the property, or raise an exception.
+    They are intended to allow validation and coercion of values, as well as
+    allowing synchronisation, for example synchronising the value of a setting with
+    a piece of hardware.
+
     If the method raises an exception, the property will not change
     its value, and the error will propagate.
 
     Side effects should be brief: they are performed synchronously
     during HTTP request handling, so should not exceed a fraction
-    of a second.
+    of a second. This is similar to the constraint on functional property setters:
+    anything likely to take a long time should be done in an action instead.
 
     :param property_name: the name of the property to which we are
         attaching a side effect.

From 77f68cb1c5f786037f33b8c63164090c01b302e6 Mon Sep 17 00:00:00 2001
From: Richard Bowman <richard.bowman@cantab.net>
Date: Tue, 26 May 2026 15:47:29 +0100
Subject: [PATCH 4/9] Tidy naming convention for `lt.on_set` functions.

This now matches the test code.

I've also improved testing and error checking, so that naming errors
give a custom exception, and said exception should happen earlier
with a stack trace pointing to the problematic method.

I've added tests of the on_set descriptor's __get__ method too, to ensure it works as expected.
---
 docs/source/properties.rst          |  3 ++-
 src/labthings_fastapi/properties.py | 22 +++++++++++++++-------
 tests/test_property.py              | 13 +++++++++++--
 3 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/docs/source/properties.rst b/docs/source/properties.rst
index 987e5303..11ea4591 100644
--- a/docs/source/properties.rst
+++ b/docs/source/properties.rst
@@ -75,7 +75,7 @@ To do this, you should use the `lt.on_set` decorator as shown below:
         """A property that holds an integer value."""
 
         @lt.on_set("my_property")
-        def _my_property_was_set(self, value: int):
+        def _on_set_my_property(self, value: int):
             """Take action because my_property was set."""
             self._hardware.set_my_property(value)
             return value
@@ -88,6 +88,7 @@ There are a few important points to note when using `lt.on_set` in your code:
 * It's ok to communicate with hardware, but you are likely to need to acquire any locks you need manually.
 * If global locking is enabled, the global lock will already have been acquired when your function is run: there's no need to acquire it again.
 * You do not need to "remember" the value as you would for a regular Python property - the data property already takes care of that.
+* You must not use the name of the property as the name of the function: this will overwrite the property and cause an error.
 
 Functional properties
 -------------------------
diff --git a/src/labthings_fastapi/properties.py b/src/labthings_fastapi/properties.py
index dbee62ec..b61127cd 100644
--- a/src/labthings_fastapi/properties.py
+++ b/src/labthings_fastapi/properties.py
@@ -912,8 +912,17 @@ def __init__(
 
         :param property_name: the name of the property we're attaching a side-effect to.
         :param func: the function to run when the property is set.
+        :raises PropertyRedefinitionError: if the `lt.on_set` function has the same name
+            as its property. This is not allowed, as it will cause the property to be
+            overwritten.
         """
         super().__init__()
+        if func.__name__ == property_name:
+            # Note: this is also checked in __set_name__, but it raises a more helpful
+            # error if it's checked here.
+            msg = f"On-set function '{property_name}' overwrites its property: "
+            msg += "rename it."
+            raise PropertyRedefinitionError(msg)
         self.property_name = property_name
         self.func = func
 
@@ -925,20 +934,19 @@ def __set_name__(self, owner: type[Owner], name: str) -> None:
 
         :param owner: the class on which we are defined.
         :param name: the name to which this descriptor is assigned.
-        :raises AttributeError: if the specified property name is missing,
-            not a data property, assigned to multiple times, or overwritten by
-            this descriptor.
+        :raises AttributeError: if the specified property name is missing or
+            not a data property.
+        :raises PropertyRedefinitionError: if the specified property already has
+            an `lt.on_set` method.
         """
-        if self.property_name == name:
-            msg = f"On-set function '{name}' overwrites its property: rename it."
-            raise AttributeError(msg)
         prop = getattr(owner, self.property_name, None)
         if not isinstance(prop, DataProperty):
             msg = "On-set functions may only be attached to data properties. "
             msg += f"'{self.property_name}' is not a data property"
             raise AttributeError(msg)
         if prop.on_set_func is not None:
-            raise AttributeError(f"'{self.property_name}.on_set' has already been set.")
+            msg = f"'{self.property_name}.on_set' has already been set."
+            raise PropertyRedefinitionError(msg)
         prop.on_set_func = self.func
 
     @overload
diff --git a/tests/test_property.py b/tests/test_property.py
index 44f17772..158ca22f 100644
--- a/tests/test_property.py
+++ b/tests/test_property.py
@@ -12,8 +12,10 @@
 """
 
 from dataclasses import dataclass
+import functools
 import json
 from typing import Any
+import types
 import warnings
 
 import fastapi
@@ -718,6 +720,13 @@ def _on_set_intprop(self, val: int) -> int:
     with pytest.raises(ValueError, match="Can't be negative"):
         thing.intprop = -1
 
+    # Accessing the function on a class should return the function
+    assert isinstance(Example._on_set_intprop, types.FunctionType)
+    assert Example._on_set_intprop.__name__ == "_on_set_intprop"
+    # Accessing it on an instance should return a bound partial object
+    assert isinstance(thing._on_set_intprop, functools.partial)
+    assert thing._on_set_intprop.args == (thing,)
+
 
 def test_bad_on_set_definitions():
     """Test that helpful errors are raise if `on_set` is used incorrectly."""
@@ -730,7 +739,7 @@ def set_missing(self, value):
 
     assert "'missing' is not a data property" in str(excinfo)
 
-    with raises_or_is_caused_by(AttributeError) as excinfo:
+    with raises_or_is_caused_by(PropertyRedefinitionError) as excinfo:
 
         class Example3(lt.Thing):
             @lt.on_set("myprop")
@@ -739,7 +748,7 @@ def myprop(self, value):
 
     assert "On-set function 'myprop' overwrites its property" in str(excinfo)
 
-    with raises_or_is_caused_by(AttributeError) as excinfo:
+    with raises_or_is_caused_by(PropertyRedefinitionError) as excinfo:
 
         class Example4(lt.Thing):
             intprop: int = lt.property(default=0)

From 7e4535d1eef479d616df48abc5a36799c404a736 Mon Sep 17 00:00:00 2001
From: Richard Bowman <richard.bowman@cantab.net>
Date: Tue, 26 May 2026 15:50:31 +0100
Subject: [PATCH 5/9] Parametrize test for on_set to add settings.

---
 tests/test_property.py | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/tests/test_property.py b/tests/test_property.py
index 158ca22f..9dbdc9f0 100644
--- a/tests/test_property.py
+++ b/tests/test_property.py
@@ -697,11 +697,16 @@ def myprop(self) -> None:
                 pass
 
 
-def test_on_set():
-    """Test that `on_set` works as expected."""
+@pytest.mark.parametrize("prop_or_setting", [lt.property, lt.setting])
+def test_on_set(prop_or_setting):
+    """Test that `on_set` works as expected.
+
+    Note that this test is parametrised so that it checks both properties
+    and settings.
+    """
 
     class Example(lt.Thing):
-        intprop: int = lt.property(default=0)
+        intprop: int = prop_or_setting(default=0)
 
         shadow: int = lt.property(default=0)
 

From 8dfdf37738e95fe8406cfb0eb23a60b1fd1a2cce Mon Sep 17 00:00:00 2001
From: Richard Bowman <richard.bowman@cantab.net>
Date: Tue, 2 Jun 2026 21:33:41 +0100
Subject: [PATCH 6/9] Implement suggestions from code review by @bprobert97

I think these are all pretty uncontroversial and resolve most
of the comments.

I'll save checking it does return a value for the next commit.
---
 src/labthings_fastapi/properties.py | 17 +++++++++--------
 tests/test_property.py              | 17 +++++++++++++++++
 2 files changed, 26 insertions(+), 8 deletions(-)

diff --git a/src/labthings_fastapi/properties.py b/src/labthings_fastapi/properties.py
index b61127cd..01369443 100644
--- a/src/labthings_fastapi/properties.py
+++ b/src/labthings_fastapi/properties.py
@@ -870,8 +870,8 @@ def on_set(
     See the description at :ref:`properties_on_set` for an example.
 
     This decorator causes a method to be called whenever a property
-    is set. The method must return the value (and may modify it), but
-    is not responsible for "remembering" the value: that's done by
+    is set. The method must return the value (and may modify it), or raise an exception,
+    but is not responsible for "remembering" the value: that's done by
     the data property.
 
     `on_set` methods should have only ``self`` and the property value as arguments,
@@ -920,9 +920,9 @@ def __init__(
         if func.__name__ == property_name:
             # Note: this is also checked in __set_name__, but it raises a more helpful
             # error if it's checked here.
-            msg = f"On-set function '{property_name}' overwrites its property: "
-            msg += "rename it."
-            raise PropertyRedefinitionError(msg)
+            raise PropertyRedefinitionError(
+                f"On-set function '{property_name}' overwrites its property: rename it."
+            )
         self.property_name = property_name
         self.func = func
 
@@ -941,9 +941,10 @@ def __set_name__(self, owner: type[Owner], name: str) -> None:
         """
         prop = getattr(owner, self.property_name, None)
         if not isinstance(prop, DataProperty):
-            msg = "On-set functions may only be attached to data properties. "
-            msg += f"'{self.property_name}' is not a data property"
-            raise AttributeError(msg)
+            raise AttributeError(
+                "On-set functions may only be attached to data properties. "
+                f"'{self.property_name}' is not a data property"
+            )
         if prop.on_set_func is not None:
             msg = f"'{self.property_name}.on_set' has already been set."
             raise PropertyRedefinitionError(msg)
diff --git a/tests/test_property.py b/tests/test_property.py
index 9dbdc9f0..a5c62069 100644
--- a/tests/test_property.py
+++ b/tests/test_property.py
@@ -709,6 +709,13 @@ class Example(lt.Thing):
         intprop: int = prop_or_setting(default=0)
 
         shadow: int = lt.property(default=0)
+        """A property that takes the same value as `intprop`.
+        
+        This property gets updated by the `on_set` function attached to
+        `intprop` such that the two properties always have the same value.
+        
+        It allows us to check that `_on_set_intprop` was run.
+        """
 
         @lt.on_set("intprop")
         def _on_set_intprop(self, val: int) -> int:
@@ -724,6 +731,9 @@ def _on_set_intprop(self, val: int) -> int:
     assert thing.shadow == 42
     with pytest.raises(ValueError, match="Can't be negative"):
         thing.intprop = -1
+    # The previous value should remain if the `on_set` function errored.
+    assert thing.intprop == 42
+    assert thing.shadow == 42
 
     # Accessing the function on a class should return the function
     assert isinstance(Example._on_set_intprop, types.FunctionType)
@@ -736,6 +746,8 @@ def _on_set_intprop(self, val: int) -> int:
 def test_bad_on_set_definitions():
     """Test that helpful errors are raise if `on_set` is used incorrectly."""
     with raises_or_is_caused_by(AttributeError) as excinfo:
+        # If the property name doesn't exist or isn't a data property, we will
+        # get an error: it must refer to a data property.
 
         class Example2(lt.Thing):
             @lt.on_set("missing")
@@ -745,6 +757,9 @@ def set_missing(self, value):
     assert "'missing' is not a data property" in str(excinfo)
 
     with raises_or_is_caused_by(PropertyRedefinitionError) as excinfo:
+        # The on_set function can't have the same name as the property.
+        # If it does, we'll overwrite the property and delete it, so we
+        # check for this and raise an error in the decorator.
 
         class Example3(lt.Thing):
             @lt.on_set("myprop")
@@ -754,6 +769,8 @@ def myprop(self, value):
     assert "On-set function 'myprop' overwrites its property" in str(excinfo)
 
     with raises_or_is_caused_by(PropertyRedefinitionError) as excinfo:
+        # You can only have on `on_set` function: decorating a second function
+        # will result in an error.
 
         class Example4(lt.Thing):
             intprop: int = lt.property(default=0)

From 5d66d6ad306040499f3d7a55da8d3f974f370c2f Mon Sep 17 00:00:00 2001
From: Richard Bowman <richard.bowman@cantab.net>
Date: Tue, 2 Jun 2026 21:59:13 +0100
Subject: [PATCH 7/9] Attempt to improve one of the bullet points about
 `on_set`.

---
 docs/source/properties.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/source/properties.rst b/docs/source/properties.rst
index 11ea4591..9d25c6d2 100644
--- a/docs/source/properties.rst
+++ b/docs/source/properties.rst
@@ -87,7 +87,7 @@ There are a few important points to note when using `lt.on_set` in your code:
 * Your function will run every time the property is set, meaning it should complete quickly. If this function takes longer than a second, it is likely to cause HTTP timeouts.
 * It's ok to communicate with hardware, but you are likely to need to acquire any locks you need manually.
 * If global locking is enabled, the global lock will already have been acquired when your function is run: there's no need to acquire it again.
-* You do not need to "remember" the value as you would for a regular Python property - the data property already takes care of that.
+* There is no need to store the value in a private attribute or provide a getter function.
 * You must not use the name of the property as the name of the function: this will overwrite the property and cause an error.
 
 Functional properties

From 0c0aef9334150e1949f01a5aba71a897fa6961d6 Mon Sep 17 00:00:00 2001
From: Richard Bowman <richard.bowman@cantab.net>
Date: Tue, 2 Jun 2026 22:08:42 +0100
Subject: [PATCH 8/9] Add some checks on on_set functions

This should help catch on_set functions that don't return a value.
---
 src/labthings_fastapi/properties.py | 19 ++++++++++++
 tests/test_property.py              | 45 ++++++++++++++++++++++++++---
 2 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/src/labthings_fastapi/properties.py b/src/labthings_fastapi/properties.py
index 01369443..cadf3d9a 100644
--- a/src/labthings_fastapi/properties.py
+++ b/src/labthings_fastapi/properties.py
@@ -49,6 +49,7 @@ class attribute. Documentation is in strings immediately following the
 import builtins
 from collections.abc import Mapping
 from functools import partial
+import inspect
 from types import EllipsisType
 from typing import (
     Annotated,
@@ -782,7 +783,15 @@ def __set__(
             self.use_global_lock
         ):
             if self.on_set_func:
+                original_value = value
                 value = self.on_set_func(obj, value)
+                # This warning tries to make it easier to spot if the on_set
+                # function has forgotten to return a value.
+                if value is None and original_value is not None:
+                    obj.logger.warning(
+                        f"{self.name}.on_set modified the value from "
+                        f"'{original_value}' to None."
+                    )
             if get_validate_properties_on_set(obj.__class__):
                 property_info = self.descriptor_info(obj)
                 value = property_info.validate(value)
@@ -915,6 +924,9 @@ def __init__(
         :raises PropertyRedefinitionError: if the `lt.on_set` function has the same name
             as its property. This is not allowed, as it will cause the property to be
             overwritten.
+        :raises MissingTypeError: if there's no type hint for the return value. This is
+            the closest we can get to checking that the function returns a value,
+            which is required for `~lt.on_set` to work properly.
         """
         super().__init__()
         if func.__name__ == property_name:
@@ -923,6 +935,13 @@ def __init__(
             raise PropertyRedefinitionError(
                 f"On-set function '{property_name}' overwrites its property: rename it."
             )
+        sig = inspect.signature(func)
+        if sig.return_annotation == inspect.Signature.empty:
+            raise MissingTypeError(
+                f"On-set function '{func.__name__}' does not have a return type "
+                "annotation. On-set functions must return a value, so an annotation "
+                "is required."
+            )
         self.property_name = property_name
         self.func = func
 
diff --git a/tests/test_property.py b/tests/test_property.py
index a5c62069..65fe4e91 100644
--- a/tests/test_property.py
+++ b/tests/test_property.py
@@ -743,6 +743,30 @@ def _on_set_intprop(self, val: int) -> int:
     assert thing._on_set_intprop.args == (thing,)
 
 
+def test_on_set_none_warning(caplog):
+    """Check that an on_set function returning `None` raises a warning."""
+
+    class Example(lt.Thing):
+        intprop: int = lt.property(default=0)
+
+        @lt.on_set("intprop")
+        def _on_set_intprop(self, val) -> int:
+            """A function to run when intprop is set."""
+            pass  # deliberately missing a return statement
+
+    example = create_thing_without_server(Example)
+    example.intprop = 42
+
+    assert example.intprop is None
+    assert caplog.record_tuples == [
+        (
+            "labthings_fastapi.things.example",
+            30,
+            "intprop.on_set modified the value from '42' to None.",
+        )
+    ]
+
+
 def test_bad_on_set_definitions():
     """Test that helpful errors are raise if `on_set` is used incorrectly."""
     with raises_or_is_caused_by(AttributeError) as excinfo:
@@ -751,7 +775,7 @@ def test_bad_on_set_definitions():
 
         class Example2(lt.Thing):
             @lt.on_set("missing")
-            def set_missing(self, value):
+            def set_missing(self, value) -> Any:
                 return value
 
     assert "'missing' is not a data property" in str(excinfo)
@@ -763,7 +787,7 @@ def set_missing(self, value):
 
         class Example3(lt.Thing):
             @lt.on_set("myprop")
-            def myprop(self, value):
+            def myprop(self, value) -> Any:
                 return value
 
     assert "On-set function 'myprop' overwrites its property" in str(excinfo)
@@ -776,11 +800,24 @@ class Example4(lt.Thing):
             intprop: int = lt.property(default=0)
 
             @lt.on_set("intprop")
-            def set_intprop(self, value):
+            def set_intprop(self, value) -> int:
                 return value
 
             @lt.on_set("intprop")
-            def set_intprop2(self, value):
+            def set_intprop2(self, value) -> int:
                 return value
 
     assert "'intprop.on_set' has already been set" in str(excinfo)
+
+    with raises_or_is_caused_by(MissingTypeError) as excinfo:
+        # on_set functions must have a type hint, as this encourages mypy
+        # to ensure they do return a value.
+
+        class Example5(lt.Thing):
+            intprop: int = lt.property(default=0)
+
+            @lt.on_set("intprop")
+            def set_intprop(self, value):
+                pass
+
+    assert "On-set function 'set_intprop' does not have a return type" in str(excinfo)

From 3d7b9faea1d0f5afb833021da6569584a3e0fe16 Mon Sep 17 00:00:00 2001
From: Richard Bowman <richard.bowman@cantab.net>
Date: Wed, 3 Jun 2026 13:53:38 +0100
Subject: [PATCH 9/9] Update tests/test_property.py

Co-authored-by: Beth <167304066+bprobert97@users.noreply.github.com>
---
 tests/test_property.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/test_property.py b/tests/test_property.py
index 65fe4e91..b0bab740 100644
--- a/tests/test_property.py
+++ b/tests/test_property.py
@@ -810,7 +810,7 @@ def set_intprop2(self, value) -> int:
     assert "'intprop.on_set' has already been set" in str(excinfo)
 
     with raises_or_is_caused_by(MissingTypeError) as excinfo:
-        # on_set functions must have a type hint, as this encourages mypy
+        # on_set functions must have a return type hint, as this encourages mypy
         # to ensure they do return a value.
 
         class Example5(lt.Thing):