Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Special handling of descriptor field like in dataclasses #1232

Open
getzze opened this issue Feb 2, 2024 · 6 comments
Open

Special handling of descriptor field like in dataclasses #1232

getzze opened this issue Feb 2, 2024 · 6 comments
Labels
dataclasses dataclass features we're lacking Feature

Comments

@getzze
Copy link

getzze commented Feb 2, 2024

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
@hynek
Copy link
Member

hynek commented Feb 3, 2024

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 hynek added the Feature label Feb 3, 2024
@getzze
Copy link
Author

getzze commented Feb 3, 2024 via email

@Tinche
Copy link
Member

Tinche commented Feb 4, 2024

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

@hynek
Copy link
Member

hynek commented Feb 4, 2024

Yes, it's very irritating.

@getzze
Copy link
Author

getzze commented Feb 4, 2024 via email

@hynek hynek added the dataclasses dataclass features we're lacking label Feb 21, 2024
@stephenprater
Copy link

stephenprater commented Jun 24, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dataclasses dataclass features we're lacking Feature
Projects
None yet
Development

No branches or pull requests

4 participants