Skip to content

Commit 25a42e5

Browse files
committed
Better handling of differently named setters.
After writing #164 I realised it was much cleaner to simply avoid returning a descriptor from `FunctionalProperty.setter` if the names didn't match. This means there will only ever be one descriptor object so we don't confuse code that looks for descriptors.
1 parent 0473952 commit 25a42e5

File tree

3 files changed

+19
-53
lines changed

3 files changed

+19
-53
lines changed

src/labthings_fastapi/base_descriptor.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,9 @@ def __set_name__(self, owner: type[Thing], name: str) -> None:
194194
195195
:param owner: the `.Thing` subclass to which we are being attached.
196196
:param name: the name to which we have been assigned.
197+
198+
:raises DescriptorAddedToClassTwiceError: if the descriptor has been
199+
assigned to two class attributes.
197200
"""
198201
if self._set_name_called:
199202
raise DescriptorAddedToClassTwiceError(

src/labthings_fastapi/thing_property.py

Lines changed: 13 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -689,57 +689,6 @@ def __init__(
689689
self._fset: ValueSetter | None = None
690690
self.readonly: bool = True
691691

692-
def __set_name__(self, owner: type[Thing], name: str) -> None:
693-
r"""Check for double-assignment and avoid confusing behaviour.
694-
695-
This function enables code that looks like the following:
696-
697-
.. code-block:: python
698-
699-
import labthings_fastapi as lt
700-
701-
702-
class Example(lt.Thing):
703-
"An example class with a property"
704-
705-
@lt.property
706-
def prop(self) -> bool:
707-
"An example property."
708-
return True
709-
710-
@prop.setter
711-
def set_prop(self, val: bool) -> None:
712-
"A setter that is not named `prop` to avoid mypy errors"
713-
pass
714-
715-
``mypy`` does not allow custom property-like descriptors to follow the
716-
syntax used by the built-in ``property`` of giving both the getter and
717-
setter functions the same name: this causes an error because it is
718-
a redefinition. We suggest using a different name for the setter to
719-
work around this. However, that then means the same descriptor appears
720-
as two different attributes, e.g. ``prop1`` and ``set_prop1``\ .
721-
722-
This function tests for that special case, and uses the first
723-
(i.e. getter's) name for :ref:`gen_docs` and does not raise an error.
724-
725-
See `.DescriptorAddedToClassTwiceError` for more details.
726-
727-
:param owner: the `.Thing` on which we are defined.
728-
:param name: the name of the attribute to which we are assigned.
729-
"""
730-
if self._set_name_called:
731-
if (
732-
owner.__qualname__ == self._owner_name
733-
and self.fset
734-
and name == self.fset.__name__
735-
and self.name == self.fget.__name__
736-
):
737-
# `__set_name__` is being called because of a differently-named
738-
# setter. This should be allowed. We won't call super, as it has
739-
# already been called for the getter
740-
return
741-
return super().__set_name__(owner, name)
742-
743692
@builtins.property
744693
def fget(self) -> ValueGetter: # noqa: DOC201
745694
"""The getter function.""" # noqa: D401
@@ -765,7 +714,7 @@ def getter(self, fget: ValueGetter) -> Self:
765714
return self
766715

767716
def setter(self, fset: ValueSetter) -> Self:
768-
"""Set the setter function of the property.
717+
r"""Set the setter function of the property.
769718
770719
This function returns the descriptor, so it may be used as a decorator.
771720
@@ -818,9 +767,21 @@ def set_myprop(self, val: int) -> None:
818767
must be silenced manually. We suggest using a different name for the setter
819768
as an alternative to adding ``# type: ignore[no-redef]`` to the setter
820769
function.
770+
771+
It will cause problems elsewhere in the code if descriptors are assigned
772+
to more than one attrubute, and this is checked in
773+
`.BaseDescriptor.__set_name__`\ . We therefore return the setter rather
774+
than the descriptor if the names don't match. The type hint does not
775+
reflect this, as it would cause problems when the names do match (the
776+
descriptor would become a ``FunctionalProperty | Callable`` and thus
777+
typing errors would happen whenever it's accessed).
821778
"""
822779
self._fset = fset
823780
self.readonly = False
781+
if fset.__name__ != self.fget.__name__:
782+
# Don't return the descriptor if it's named differently.
783+
# see typing notes in docstring.
784+
return fset # type: ignore[return-value]
824785
return self
825786

826787
def instance_get(self, obj: Thing) -> Value:

tests/test_property.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,5 +247,7 @@ def set_prop(self, val: bool) -> None:
247247
"""A setter named differently."""
248248
pass
249249

250+
assert isinstance(Example.prop, tp.FunctionalProperty)
250251
assert Example.prop.name == "prop"
251-
assert Example.prop is Example.set_prop
252+
assert not isinstance(Example.set_prop, tp.FunctionalProperty)
253+
assert callable(Example.set_prop)

0 commit comments

Comments
 (0)