attrs icon indicating copy to clipboard operation
attrs copied to clipboard

Special handling of descriptor field like in dataclasses

Open getzze opened this issue 1 year ago • 6 comments
trafficstars

dataclass dataclasses handle descriptor fields differently from other fields, so the descriptor type is not lost when assigning: https://docs.python.org/3/library/dataclasses.html#descriptor-typed-fields

This does not work in attrs, it's a pity because it could be used as an alternative to converters called with the instance (#1108). Previous discussion about this was not very conclusive (#881).

# Example from python docs
from dataclasses import dataclass
from attrs import define


class IntConversionDescriptor:
    def __init__(self, *, default):
        self._default = default

    def __set_name__(self, owner, name):
        self._name = "_" + name

    def __get__(self, obj, type):
        if obj is None:
            return self._default

        return getattr(obj, self._name, self._default)

    def __set__(self, obj, value):
        setattr(obj, self._name, int(value))

# dataclass: WORKS AS EXPECTED
@dataclass
class InventoryItem:
    quantity_on_hand: IntConversionDescriptor = IntConversionDescriptor(default=100)

i = InventoryItem()
print(i.quantity_on_hand)   # 100
i.quantity_on_hand = 2.5    # calls __set__ with 2.5
print(i.quantity_on_hand)   # 2


# attrs: DOES NOT WORK
@define
class InventoryItem2:
    quanttity_on_hand: IntConversionDescriptor = IntConversionDescriptor(default=100)

i2 = InventoryItem2()
print(i2.quantity_on_hand)   # <__main__.IntConversionDescriptor object at 0x78c3a12a1250>
i2.quantity_on_hand = 2.5    # set InventoryItem2 attribute to a float, erasing the descriptor
print(i2.quantity_on_hand)   # 2.5

getzze avatar Feb 02 '24 11:02 getzze

I was wondering when they added it, because I didn't have it on my radar at all, but it looks like 3.10 – if the docs are to be trusted (3.9 doesn't mention descriptors at all).

Does anyone have a clue how much work it would be to integrate this?

hynek avatar Feb 03 '24 07:02 hynek

I checked the source code of dataclasses and the only mention of descriptors is in this line: https://github.com/python/cpython/blob/718dbd4cd33a204e12fb747a09b73c4c4ac1b55f/Lib/dataclasses.py#L349

Maybe defining the method __set_name__ is enough to instantiate the descriptor and make it work.

On 3 February 2024 08:02:38 GMT+01:00, Hynek Schlawack @.***> wrote:

I was wondering when they added it, because I didn't have it on my radar at all, but it looks like 3.10 – if the docs are to be trusted (3.9 doesn't mention descriptors at all).

Does anyone have a clue how much work it would be to integrate this?

-- Reply to this email directly or view it on GitHub: https://github.com/python-attrs/attrs/issues/1232#issuecomment-1925181425 You are receiving this because you authored the thread.

Message ID: @.***>

getzze avatar Feb 03 '24 10:02 getzze

I'm in favor of fixing this on philosophical grounds (I'm bothered by dataclasses doing a thing better than us) ;)

Tinche avatar Feb 04 '24 16:02 Tinche

Yes, it's very irritating.

hynek avatar Feb 04 '24 17:02 hynek

Maybe it's not too hard to implement, because annotating the descriptor as ClassVar is enough to make it work:


# attrs: THIS WORKS
from typing import ClassVar

@define(slots=False)
class InventoryItem3:
    quantity_on_hand: ClassVar[IntConversionDescriptor] = IntConversionDescriptor(default=100)

i3 = InventoryItem3()
print(i3.quantity_on_hand) # 100
i3.quantity_on_hand = 2.5
print(i3.quantity_on_hand) # 2

On 4 February 2024 18:04:49 GMT+01:00, Hynek Schlawack @.***> wrote:

Yes, it's very irritating.

-- Reply to this email directly or view it on GitHub: https://github.com/python-attrs/attrs/issues/1232#issuecomment-1925838777 You are receiving this because you authored the thread.

Message ID: @.***>

getzze avatar Feb 04 '24 22:02 getzze

The problem with setting them as ClassVars is that they are no longer available in __init__ - only to being set at runtime.

I don't think it would be that difficult to implement the same kind of special treament as Dataclass. I managed to make it work by doing some hacking in a field_transformer. The trick is to let the runtime handle the calls to this particular attr. (I believe that's why @getzze's "ClassVar" trick works - because it doesn't add things defined on the class to the attrs list.) Instead attributes using descriptors store the descriptor instance on the class itself and then delegate from the instance with the properties to the descriptor instance

import pytest
from attrs import define, field
from attrs.exceptions import FrozenInstanceError
from attr import Attribute
import attrs
from dataclasses import dataclass, field as dc_field

class Descriptor:
    def __set_name__(self, owner, name):
        print(f"setting #{owner} #{name}")
        self.name = name
        self.private_name = f"_{name}"

    def __get__(self, instance, owner):
        print(f"Getting #{instance} #{owner}")
        if instance is None:
            return self
        attr = getattr(instance, self.private_name, None)
        if attr is None or attr is self:
            return None
        return attr

    def __set__(self, instance, value):
        print(f"Setting #{instance} #{value}")
        setattr(instance, self.private_name, value)

    def raise_on_used(self):
        raise ValueError("Tried to use a field not set")


def test_descriptor_with_attrs():
    def use_descriptor(
            cls: type,
            fields: list[attrs.Attribute]
            ) -> list[attrs.Attribute]:
        new_fields = []
        for def_field in fields:
            descriptor = def_field.type
            if (hasattr(descriptor, "__get__") or
                hasattr(descriptor, "__set__") or
                hasattr(descriptor, "__delete__")):
                if not hasattr(descriptor, "__set_name__"):
                    raise ValueError("Descriptor must have __set_name__ to work with this transformer")
                descriptor_instance = descriptor() #type: ignore
                getattr(descriptor_instance, "__set_name__")(cls, def_field.name)
                setattr(cls, def_field.name, descriptor_instance)
                # create a "shadow" field that accepts the value in the init
                ca = field(
                        init=True,
                        repr=False,
                        default=None,
                    )
                a = Attribute.from_counting_attr( #type: ignore
                        name=f"_{def_field.name}",
                        ca=ca,
                        type="Optional[Any]",
                    )
                new_fields.append(a)
            else:
                new_fields.append(def_field)
        return new_fields

    @define(slots=False, field_transformer=use_descriptor)
    class Demo:
        int_field: int
        descriptor_field: Descriptor = Descriptor()

    demo = Demo(int_field=1)

    assert demo.descriptor_field is None

    demo2 = Demo(int_field=2, descriptor_field=2)
    assert demo2.descriptor_field == 2

def test_descriptor_with_dataclass():
    @dataclass()
    class Demo:
        int_field: int
        descriptor_field: Descriptor = Descriptor()

    demo = Demo(int_field=1)

    assert demo.descriptor_field is None


    demo2 = Demo(int_field=2, descriptor_field=2)
    assert demo2.descriptor_field == 2

The "unsolved" part of this with this hack is actually around repr - For attrs the "shadow" field that is passed to init doesn't appear in the repr output - but neither does the actual field - because it's not described in the field list. (Dataclass has a similar problem - in that repr calls the descriptor get method - so you need to make sure that get method doesn't do anything untoward.)

That seems relatively solvable - special case "descriptor" fields in the generated repr method - but I don't think it's possible to hack it in.

~I'm not sure this approach would work with __slots__ - it seems broadly difficult to make slotted classes place nicely with descriptors.~ actually seems to work the same even with slots = True

If you're okay with an approach similar to this I'm happy to send a PR. I haven't done much checking into edge cases though, so not sure what the knock-on effects would be.

stephenprater avatar Jun 24 '24 21:06 stephenprater