Type Hinting in descriptors and decorators for Thing Properties and settings

[julianstirling]

There was previously a TODO in the @thing_property decorator saying to try the following:
https://stackoverflow.com/questions/54413434/type-hinting-with-descriptors

@rwb27 Are you able to provide more details about the issue that needs to be solved?

[rwb27]

Currently, the type hints on ThingProperty are pretty loose. We have a model that's correct for serialising/deserialising and for generating the OpenAPI and Thing Description docs. However, that is a run time variable and doesn't help e.g. mypy to check types within the Thing code.

I think the nice solution is to make ThingProperty a generic class, so it's __get__ and __set__ methods can have the intended type. This would enable mypy to do its job and catch a bunch of errors that are currently invisible.

This would most likely change the syntax slightly, as it's hard to write using the current arguments (though I'll try a couple more things before giving up). Specifically it might be necessary to get rid of the model argument and instead use a subscript:

class ExampleThing(Thing):
    count = ThingProperty (default=0, model=int)

Becomes

class ExampleThing(Thing):
    count = ThingProperty[int](default=0)

I prefer this anyway as it makes it clear int is a type hint and avoids an argument that really wants to be called type but can't be.

The other option is to use dataclass syntax and say

class ExampleThing(Thing):
    count: int = ThingProperty(default=0)

This results in a descriptor that's not type hinted, but the attribute is typed correctly when it's accessed. That means code using the property will be correctly type checked. It is even possible this will be picked up by more auto completion/documentation tools than the descriptor. Both the standard dataclasses library and pydantic do this. It is probably necessary to wrap the class in a function so we can use @overload to be more explicit about typing, and to stop mypy flagging the assignment of a descriptor to something type hinted as a value.

It does feel a little like this solution is confusing mypy into accepting something that's not quite right - but the types that matter are correct, and it's an established pattern used by popular libraries.

It's absolutely possible to combine both options in one class, so you could take your pick of

class ExampleThing(Thing):
    count = ThingProperty[int](default=0)
    quantity: int = thing_property(default=0)

    @thing_property
    def number(self) -> int:
        ...

Note that, if the type of the default value is obvious, the first option doesn't actually need the subscript - though we might encourage it anyway.

I realise the first two (count and quantity) are pretty close to equivalent. It may be undesirable to have both, or we may want to strongly prefer one of them for consistency. Which one we recommend probably ought to depend on support from type checkers/auto completion and on how intuitive the syntax feels.

[rwb27]

After some testing, I think it only works without the subscript if we use a wrapper function. I'm also not convinced inferring the role from the default is something to encourage, so perhaps that's one to avoid.

[rwb27]

I've mocked up a simplified version of this to play with type checking outside of the main repo: for completeness I'll put it in here.

I think a nice solution is to split ThingProperty in two: a base class that subclasses property, and a subclass that implements the variable-like behaviour. I have called them BaseThingProperty and ThingProperty but there's room for discussion. Bear in mind that BaseThingProperty will more or less always be created using a decorator, probably that decorator should be a function with the usual snake case (though it's possible one could use BaseThingProperty directly).

Currently, I've verified that mypy passes on this, and that all three properties end up with the correct model. However, each has plusses and minuses...

class MyThing(SimpleThing):
    type_on_attr: str | None = thing_property(default='my_property_default')
    """This is a docstring for my property.""" # might work with https://davidism.com/attribute-docstrings/
    type_from_subscript = ThingProperty[str | None](default=None)
    @thing_property
    def type_from_getter(self) -> str | None:
        """This is a docstring for my getter property."""
        return 'my_property_from_getter'

mything = MyThing()

Looking at VSCode autocompletion, type_on_attr is correctly type hinted as str | None. Unfortunately, type_from_subscript is type hinted as ThingProperty[str] which isn't correct for attribute access, and type_from_getter is just Any, despite my efforts.

mypy reveal_type is correct for all three options, which is good.

Click to see some code

import typing
from typing import Any, Callable, Generic, Self, TypeVar, overload
from weakref import WeakKeyDictionary

class Missing:
    """A class to represent a missing value."""
    def __repr__(self) -> str:
        return "MISSING"

MISSING = Missing()

class SimpleThing:
    pass

class MissingTypeError(TypeError):
    """Error raised when a type annotation is missing for a property."""

class MismatchedTypeError(TypeError):
    """Error raised when a type annotation does not match the expected type for a property."""

ThingType = TypeVar('ThingType', bound=SimpleThing)
ValType = TypeVar('ValType')

class BaseThingProperty(property, Generic[ValType]):
    model: type[ValType]  # Default to Any, will be set in __set_name__
    _model_arg: type[ValType]

    # def __init__(
    #         self,
    #         fget: Callable[[ThingType], ValType] | None = None,
    #         fset: Callable[[ThingType, ValType], None] | None = None,
    #         model: type[ValType] | Missing = MISSING,
    #     ) -> None:
    #     if not isinstance(model, Missing):
    #         self._model_arg = model
    #     super().__init__(fget=fget, fset=fset)

    def __set_name__(self, owner: type[ThingType], name: str) -> None:
        self.name = name
        field_annotation = typing.get_type_hints(owner, include_extras=True).get(name, MISSING)
        models: dict[str, type] = {}
        if hasattr(self, "_model_arg"):
            # If we have a model argument, we can use that
            self.model = self._model_arg
            models["model_argument"] = self.model
        if self.fget is not None:
            # If the property has a getter, we can extract the type from it
            return_type = typing.get_type_hints(self.fget, include_extras=True).get('return', MISSING)
            if return_type is not MISSING:
                self.model = return_type
                models["getter"] = self.model
        if field_annotation is not MISSING:
            # If the property has a type annotation on the owning class, we can use that
            self.model = field_annotation
            models["class_attribute"] = self.model
        if hasattr(self, "__orig_class__"):
            # We were instantiated as BaseThingProperty[ModelType] so can use that type
            self.model = typing.get_args(self.__orig_class__)[0]
            models["__orig_class__"] = self.model

        # Check we have a model, and that it is consistent if it's specified in multiple places
        if not hasattr(self, "model"):
            raise MissingTypeError(
                f"Property '{name}' on '{owner}' is missing a type annotation. "
                "Please provide a type annotation ."
            )
        # Check all options against the final one
        for key, value in models.items():
            if value != self.model:
                raise MismatchedTypeError(
                    f"Inconsistent model for property '{name}' on '{owner}'. "
                    f"The final model is {self.model}, picked from {models}."
                )
        # Debug print statement, remove
        print(f"Initializing property '{name}' on '{owner}', found {self.model} from {models}.")
            
    def __get__(self, instance: Any, owner: type | None = None) -> ValType:
        """Get the value of the property."""
        # It's necessary to override this so it's typed correctly.
        if instance is None:
            raise AttributeError(
                f"Property '{self.name}' cannot be accessed on the class {owner}."
            )
        return super().__get__(instance, owner)


class NoCustomGetterSetterError(NotImplementedError):
    """Error raised when trying to set a custom getter or setter on a variable-like property. 
    SimpleThingProperty objects cannot have a modified getter or setter. 
    Use a ThingProperty instead.
    """

class ThingProperty(BaseThingProperty[ValType], Generic[ValType]):
    """A property for SimpleThing that can be used with type annotations."""
    default: ValType
    _values: WeakKeyDictionary[SimpleThing, ValType]

    def __init__(self, *, default: ValType) -> None:
        super().__init__()
        self.default = default
        self._values = WeakKeyDictionary()

    def __get__(self, instance: Any, owner: type | None = None) -> ValType:
        if instance is None:
            raise AttributeError(
                f"Property '{self.name}' cannot be accessed on the class {owner}."
            )
        assert isinstance(instance, SimpleThing), "ThingProperties can only be used on Things"
        # Storing values in instance.__dict__ is hard to type properly. Instead,
        # we use a WeakKeyDictionary to store values per instance. The weak keys
        # are important to avoid circular references: the instance holds a reference
        # to the property already.
        return self._values.get(instance, self.default)

    def __set__(self, instance: SimpleThing, value: ValType) -> None:
        print(f"Setting property '{self.name}' to {value} on {instance}")
        self._values[instance] = value

    def __delete__(self, instance: SimpleThing) -> None:
        """Deletion will reset the property to its default."""
        self.__set__(instance, self.default)

    def getter(self, func: Callable[[SimpleThing], ValType]) -> Self:
        """Custom getters may not be set: this raises an error."""
        raise NoCustomGetterSetterError()
    
    def setter(self, func: Callable[[SimpleThing, ValType], None]) -> Self:
        """Custom setters may not be set: this raises an error."""
        raise NoCustomGetterSetterError()
    


# @overload  # This is for use as a decorator - return a ThingProperty with the return type
# def thing_property(getter: Callable[[SimpleThing,], ValType]) -> ThingProperty[ValType]: ...
# @overload # This is field-style usage - return a ThingProperty with the default value
# def thing_property(*, default: ValType) -> ValType: ...  #ThingProperty[ValType]: ...
# def thing_property(
#         getter: Callable[[SimpleThing,], Any] | None = None,
#         default: Any = MISSING,
#     ) -> Any:
#     """Wrapper to create a property with a default value."""
#     if getter:
#         valtype = typing.get_type_hints(getter).get('return', Any)
#         return BaseThingProperty[valtype](fget=getter, model=valtype)
#     return ThingProperty(default=default)

def thing_property(
        fget: Callable[[ThingType], ValType]
) -> BaseThingProperty[ValType]:
    """Decorator to create a property with a getter."""
    return BaseThingProperty(fget=fget)

def thing_property_field(*, default: ValType) -> ValType:
    """Field-style usage to create a property with a default value."""
    return ThingProperty(default=default)  # type: ignore[return-value]


class MyThing(SimpleThing):
    type_on_attr: str | None = thing_property_field(default='my_property_default')
    """This is a docstring for my property.""" # might work with https://davidism.com/attribute-docstrings/
    type_from_subscript = ThingProperty[str | None](default=None)
    @thing_property
    def type_from_getter(self) -> str | None:
        """This is a docstring for my getter property."""
        return 'my_property_from_getter'
    @property
    def actual_property(self) -> str | None:
        """This is a docstring for my actual property."""
        return 'actual_property_value'

thing = MyThing()

for prop in ['type_on_attr', 'type_from_getter', 'type_from_subscript']:
    descriptor = MyThing.__dict__[prop]
    print(
        f"Property '{prop}':\n"
        f"    type: {type(getattr(thing, prop))}\n"
        f"    value: {getattr(thing, prop)}\n"
        f"    docstring: {descriptor.__doc__}\n"
        f"    model: {descriptor.model}\n"
    )
    

[rwb27]

This is closed by #155 .