attrs icon indicating copy to clipboard operation
attrs copied to clipboard

Why descriptor don't work with attr?

Open jalvespinto opened this issue 4 years ago • 13 comments

I am reading Architecture Patterns with Python and at some point the frozen dataclasses just don't work with sqlalchemy, so the author suggests setting unsafe_hash=True. To avoid making the class hashable I am using the descriptor bellow. I was also using @attr.s instead of @dataclass to get some of the features it has, like validators, but for some reason the descriptor works fine with dataclass, but not with attr and I can't understand why and if there is a way to make it work. If I set init=False on the attr.s class the descriptor will work for self.vid the I am only setting inside the init, but don't work with street and number.

from dataclasses import dataclass
import attr
import uuid


class Frozen:
    """cannot change attribute if it already exists"""

    def __set_name__(self, owner, name):
        self.storage_name = '_' + name

    def __set__(self, instance, value):
        if hasattr(instance, self.storage_name):
            raise AttributeError(
                f'readonly attribute {value.__class__.__name__}')
        else:
            setattr(instance, self.storage_name, value)

    def __get__(self, instance, objtype=None):
        return getattr(instance, self.storage_name)

# Frozen descriptor works fine here
@dataclass
class Address:

    vid = Frozen()
    street = Frozen()
    number = Frozen()

    street: str
    number: str

    def __init__(self, street: str, number: str, vid=None) -> None:
        self.street = street
        self.number = number
        self.vid = vid if vid else uuid.uuid4().hex

# Frozen descriptor don't work here
@attr.s
class Address:

    vid = Frozen()
    street = Frozen()
    number = Frozen()

    street: str = attr.ib
    number: str = attr.ib

    def __init__(self, street: str, number: str, vid=None) -> None:
        self.street = street
        self.number = number
        self.vid = vid if vid else uuid.uuid4().hex

jalvespinto avatar Dec 06 '21 12:12 jalvespinto

Try setting @attr.s(auto_attribs=True) (or even better: use @attr.define) and dropping the = attr.ib. You're overwriting both street and number this way.

Note that you can just use @attr.s(frozen=True) or @attr.frozen to get a frozen class.

hynek avatar Dec 06 '21 13:12 hynek

The problem with frozen is that it does not work well with sqlalchemy for some reason, at least the way I am using it (https://github.com/cosmicpython/code/issues/17).

The code bellow works. Its works with sqlalchemy and the descriptors will prevent setting the attributes. But if I use @attr.define I will get in __setattr__ _obj_setattr(self, name, nval) AttributeError: 'Address' object has no attribute '_sa_instance_state'. The same happens if I use frozen=True in both dataclass or attr.

import uuid
from dataclasses import dataclass

import attr
from sqlalchemy import Column, Table, create_engine
from sqlalchemy.orm import registry,sessionmaker
from sqlalchemy.sql.sqltypes import Integer, String


class FrozenLeadingUnder:
    """cannot change attribute if it already exists"""

    def __set_name__(self, owner, name):
        self.storage_name = '_' + name

    def __set__(self, instance, value):
        if hasattr(instance, self.storage_name):
            raise AttributeError(
                f'readonly attribute {value.__class__.__name__}')
        else:
            setattr(instance, self.storage_name, value)

    def __get__(self, instance, objtype=None):
        return getattr(instance, self.storage_name)


class Frozen:
    """cannot change attribute if it already exists"""

    def __set_name__(self, owner, name):
        self.storage_name = name

    def __set__(self, instance, value):
        if self.storage_name in instance.__dict__:
            raise AttributeError(
                f'readonly attribute {value.__class__.__name__}')
        else:
            instance.__dict__[self.storage_name] = value

    def __get__(self, instance, objtype=None):
        try:
            return instance.__dict__[self.storage_name]
        except KeyError:
            raise KeyError(f'No {self.storage_name} attribute found.')


mapper_registry = registry()


@dataclass
class Address:

    vid = FrozenLeadingUnder()
    street = Frozen()
    number = Frozen()

    street: str
    number: str

    def __init__(self, street: str, number: str, vid=None) -> None:
        self.street = street
        self.number = number
        self.vid = vid if vid else uuid.uuid4().hex


addresses = Table(
    'addresses', mapper_registry.metadata,
    Column('id', Integer, primary_key=True, autoincrement=True),
    Column('street', String(255)),
    Column('number', String(255))
)


def start_mappers():
    mapper_registry.map_imperatively(Address, addresses)


if __name__ == "__main__":

    engine = create_engine("sqlite:///:memory:")
    mapper_registry.metadata.create_all(engine)
    start_mappers()
    session = sessionmaker(bind=engine)()

    ad = Address("Av", '123')

    session.add(ad)
    session.commit()

jalvespinto avatar Dec 06 '21 14:12 jalvespinto

Ah you're using additional attributes. Try @attr.define(slots=False) then.

hynek avatar Dec 06 '21 14:12 hynek

But even without them, the descriptors won't work. The behavior I am seeing is that any field I declare will override the descriptor, so the descriptor will not work. If I don't declare the fields and just set them on init the descriptor will work, but I don't see the attribute when invoking the instance:

@attr.define
class Address:

    street = Frozen()
    number = Frozen()

    def __init__(self, street: str, number: str) -> None:
        self.street = street
        self.number = number

>>> a = Address('bla','ble')
>>> a.street = 'dfad'
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/usr/test.py", line 35, in __set__
    raise AttributeError(
AttributeError: readonly attribute str
>>> a
Address()

jalvespinto avatar Dec 06 '21 15:12 jalvespinto

You have to annotate street and number if attrs is to find them.

hynek avatar Dec 06 '21 16:12 hynek

yes, but in that case the descriptor will not work:

>>> a = Address('ba','be')
>>> a.street = 'fadfadf'
>>> a
Address(street='fadfadf', number='be')
>>> 

jalvespinto avatar Dec 06 '21 16:12 jalvespinto

I have tried:

@attr.define(slots=False)
class Address:

    street: str = Frozen()
    number: str = Frozen()
    vid: str = Frozen()

And I get:

In [2]: a = Address("x", "y", "z")

In [3]: a
Out[3]: Address(street='x', number='y', vid='z')

In [4]: a.street = "asdf"
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-4-f728b241b7ae> in <module>
----> 1 a.street = "asdf"

~/FOSS/attrs/t.py in __set__(self, instance, value)
     12     def __set__(self, instance, value):
     13         if hasattr(instance, self.storage_name):
---> 14             raise AttributeError(
     15                 f'readonly attribute {value.__class__.__name__}')
     16         else:

AttributeError: readonly attribute str

The attribute name is wrong, but it seems to work otherwise?

hynek avatar Dec 06 '21 16:12 hynek

So I'm wondering if this is a typo?

# Frozen descriptor don't work here
@attr.s
class Address:
    <snip>
    street: str = attr.ib

That should in theory be attr.ib(). otherwise attr.s is doing nothing with that and it's just overwriting the Frozen that's going on above.

euresti avatar Dec 06 '21 16:12 euresti

oh, I see. I was doing two different assignments to the same name, one for the descriptor and another for the actual attributes, just like the way I was doing with dataclass. I wonder why that works with dataclass and not with attrs. It seems slots=False will do it. I will refactor everything and then come here to let you know how it went, for good or bad. tks

jalvespinto avatar Dec 06 '21 16:12 jalvespinto

Note that this:

@attr.define(slots=False)
class Address:
    street: str = Frozen()
    number: str = Frozen()
    vid: str = Frozen()

has a problem where attrs thinks that Frozen() is a default value. So it lets you do this:

>>> Address()
Address(street=<foo.Frozen object at 0x7fab78034df0>, number=<foo.Frozen object at 0x7fab78034ca0>, vid=<foo.Frozen object at 0x7fab78034e20>)
>>> 

So you'll probably want to write your own __init__

@attr.define(slots=False)
class Address:
    street: str = Frozen()
    number: str = Frozen()
    vid: str = Frozen()

    def __init__(self, street: str, number: str, vid=None) -> None:
        self.street = street
        self.number = number
        self.vid = vid if vid else uuid.uuid4().hex

euresti avatar Dec 06 '21 17:12 euresti

noted. tks

jalvespinto avatar Dec 06 '21 17:12 jalvespinto

I am having some difficulties still. Now I had to add some more code to my example. I included a House class that has an Address as attribute and also some another Table and mapper for this new class.

import uuid
from dataclasses import dataclass

import attr
from sqlalchemy import Column, Table, ForeignKey, create_engine
from sqlalchemy.orm import registry, relationship, sessionmaker
from sqlalchemy.sql.sqltypes import Integer, String


class FrozenPrivate:
    """cannot change attribute if it already exists"""

    def __set_name__(self, owner, name):
        self.storage_name = '_' + name

    def __set__(self, instance, value):
        if hasattr(instance, self.storage_name):
            raise AttributeError(
                f'readonly attribute {value.__class__.__name__}')
        else:
            setattr(instance, self.storage_name, value)

    def __get__(self, instance, objtype=None):
        return getattr(instance, self.storage_name)


class Frozen:
    """cannot change attribute if it already exists"""

    def __set_name__(self, owner, name):
        self.storage_name = name

    def __set__(self, instance, value):
        if self.storage_name in instance.__dict__:
            raise AttributeError(
                f'readonly attribute {value.__class__.__name__}')
        else:
            instance.__dict__[self.storage_name] = value

    def __get__(self, instance, objtype=None):
        try:
            return instance.__dict__[self.storage_name]
        except KeyError:
            raise KeyError(f'No {self.storage_name} attribute found.')


mapper_registry = registry()


@attr.define(slots=False)
# @dataclass
class Address:

    street: str = Frozen()
    number: str = Frozen()
    vid: str = FrozenPrivate()

    def __init__(self, street: str, number: str, vid=None) -> None:
        self.street = street
        self.number = number
        self.vid = vid if vid else uuid.uuid4().hex


class House:

    def __init__(self, ad: Address) -> None:
        self.address = ad


houses = Table(
    'houses', mapper_registry.metadata,
    Column('id', Integer, primary_key=True, autoincrement=True),
    Column('address_id', ForeignKey('addresses.id'))
)

addresses = Table(
    'addresses', mapper_registry.metadata,
    Column('id', Integer, primary_key=True, autoincrement=True),
    Column('street', String(255)),
    Column('number', String(255))
)


def start_mappers():
    mapper_registry.map_imperatively(Address, addresses)
    mapper_registry.map_imperatively(
        House,
        houses,
        properties={'address': relationship(Address, backref='house')}
    )


if __name__ == "__main__":

    engine = create_engine("sqlite:///:memory:")
    mapper_registry.metadata.create_all(engine)
    start_mappers()
    session = sessionmaker(bind=engine)()

    ad = Address("Av", '123')
    h = House(ad)

    session.add(h)
    session.commit()

    search = session.query(House).join(House.address).filter(
        Address.street == ad.street and Address.number == ad.number).first()

    print(search.address)

It seems that the filter method don't recognize the ad variable as an Address instance:

Traceback (most recent call last):
  File "/home/jap/projects/vlep/app/bora.py", line 107, in <module>
    Address.street == ad.street and Address.number == ad.number).first()
  File "/home/jap/projects/vlep/app/bora.py", line 42, in __get__
    return instance.__dict__[self.storage_name]
AttributeError: 'NoneType' object has no attribute '__dict__'

I tried to simple delete the get method on the descriptor, but them the descriptor will not work (the query will, but not the descriptor):

Python 3.9.7 (default, Sep  9 2021, 23:20:13) 
[GCC 9.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import bora as b
>>> ad = b.Address('fdaf','fadfa')
>>> ad.street = 'fdfadfafa'
>>> ad
Address(street='fdfadfafa', number='fadfa', vid='9e032ff432844679a03ebc4010865c00')

jalvespinto avatar Dec 06 '21 18:12 jalvespinto

As a random though: I think you could use field hooks to remove Frozen as the default value: https://www.attrs.org/en/stable/extending.html#automatic-field-transformation-and-modification

maybe they could be useful to you in general.

hynek avatar Dec 12 '21 13:12 hynek